Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(31)

Issue 122740044: code review 122740044: runtime: missed a file in alg checkin (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by khr
Modified:
10 years, 7 months ago
Reviewers:
dave, khr1, dvyukov
CC:
golang-codereviews
Visibility:
Public.

Description

runtime: missed a file in alg checkin

Patch Set 1 #

Patch Set 2 : diff -r 2aa6648d7146 https://khr%40golang.org@code.google.com/p/go/ #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+153 lines, -0 lines) Patch
A src/pkg/runtime/alg.go View 1 chunk +153 lines, -0 lines 7 comments Download

Messages

Total messages: 8
khr
Hello dvyukov (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://khr%40golang.org@code.google.com/p/go/
10 years, 7 months ago (2014-07-31 22:12:52 UTC) #1
khr
*** Submitted as https://code.google.com/p/go/source/detail?r=d81fb87ee976 *** runtime: missed a file in alg checkin TBR=dvyukov CC=golang-codereviews https://codereview.appspot.com/122740044
10 years, 7 months ago (2014-07-31 22:12:56 UTC) #2
dvyukov
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go File src/pkg/runtime/alg.go (right): https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#newcode43 src/pkg/runtime/alg.go:43: func aeshash(p unsafe.Pointer, s uintptr, h uintptr) uintptr s/s ...
10 years, 7 months ago (2014-08-05 11:03:46 UTC) #3
dvyukov
I guess this needs LGTM as it's already submitted On 2014/08/05 11:03:46, dvyukov wrote: > ...
10 years, 7 months ago (2014-08-05 11:04:15 UTC) #4
dave_cheney.net
dmitry, why don't you propose a followup CL, your suggestions look good. On Tue, Aug ...
10 years, 7 months ago (2014-08-05 11:22:41 UTC) #5
dvyukov
yes, it needs a follow up cl On Tue, Aug 5, 2014 at 3:22 PM, ...
10 years, 7 months ago (2014-08-05 11:30:33 UTC) #6
khr1
Follow up in CL 117680044. We can't use iface/eface as parameter types because the definitions ...
10 years, 7 months ago (2014-08-06 20:23:19 UTC) #7
khr1
10 years, 7 months ago (2014-08-06 20:24:09 UTC) #8
Actually, I take that back - the declarations are pointers to interfaces.
 Those should be fine.



On Wed, Aug 6, 2014 at 1:23 PM, Keith Randall <khr@google.com> wrote:

> Follow up in CL 117680044.
>
> We can't use iface/eface as parameter types because the definitions of
> iface/eface use unsafe.Pointer as the data word.  But for some interfaces
> that data word may not be a pointer.
>
>
> On Tue, Aug 5, 2014 at 4:30 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>
>> yes, it needs a follow up cl
>>
>> On Tue, Aug 5, 2014 at 3:22 PM, Dave Cheney <dave@cheney.net> wrote:
>> > dmitry, why don't you propose a followup CL, your suggestions look good.
>> >
>> > On Tue, Aug 5, 2014 at 9:04 PM, dvyukov via golang-codereviews
>> > <golang-codereviews@googlegroups.com> wrote:
>> >> I guess this needs LGTM as it's already submitted
>> >>
>> >>
>> >> On 2014/08/05 11:03:46, dvyukov wrote:
>> >>
>> >>
>> https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go
>> >>>
>> >>> File src/pkg/runtime/alg.go (right):
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:43: func aeshash(p unsafe.Pointer, s uintptr, h
>> >>
>> >> uintptr)
>> >>>
>> >>> uintptr
>> >>> s/s uintptr, h uintptr/s, h uintptr/
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:45: func memhash(p unsafe.Pointer, s uintptr, h
>> >>
>> >> uintptr)
>> >>>
>> >>> uintptr {
>> >>> s/s uintptr, h uintptr/s, h uintptr/
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:59: func strhash(a *string, s uintptr, h
>> >>
>> >> uintptr) uintptr
>> >>>
>> >>> {
>> >>> s/s uintptr, h uintptr/s, h uintptr/
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:68: func f32hash(a *float32, s uintptr, h
>> >>
>> >> uintptr)
>> >>>
>> >>> uintptr {
>> >>> s/s uintptr, h uintptr/s, h uintptr/
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:80: func f64hash(a *float64, s uintptr, h
>> >>
>> >> uintptr)
>> >>>
>> >>> uintptr {
>> >>> s/s uintptr, h uintptr/s, h uintptr/
>> >>
>> >>
>> >>> here and below
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:109: func interhash(a *interface {
>> >>> can we use iface directly as parameter type?
>> >>
>> >>
>> >>
>> >>
>>
https://codereview.appspot.com/122740044/diff/20001/src/pkg/runtime/alg.go#ne...
>> >>>
>> >>> src/pkg/runtime/alg.go:115: func nilinterhash(a *interface{}, s
>> >>
>> >> uintptr, h
>> >>>
>> >>> uintptr) uintptr {
>> >>> can we use eface directly as parameter type?
>> >>
>> >>
>> >>
>> >>
>> >> https://codereview.appspot.com/122740044/
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google
>> Groups
>> >> "golang-codereviews" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send
>> an
>> >> email to golang-codereviews+unsubscribe@googlegroups.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>>
>
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b