Issue 122740044: code review 122740044: runtime: missed a file in alg checkin (Closed)
Descriptionruntime: 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
MessagesTotal messages: 8
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/
Sign in to reply to this message.
*** 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
Sign in to reply to this message.
Message was sent while issue was closed.
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?
Sign in to reply to this message.
Message was sent while issue was closed.
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?
Sign in to reply to this message.
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.
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.
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.
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.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||
