code review 66510044: cmd/gc: make embedded, unexported fields read-only.

cmd/gc: make embedded, unexported fields read-only.

Fixes issue 7363.

[cmang, 2014-02-20 18:17:32]

Hello gri@golang.org, rsc@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2014-02-20 18:19:50]

No new test?



On Thu, Feb 20, 2014 at 10:17 AM, <cmang@golang.org> wrote:

> Reviewers: gri, rsc,
>
> Message:
> Hello gri@golang.org, rsc@golang.org (cc:
> golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> cmd/gc: make embedded, unexported fields read-only.
>
> Fixes issue 7363.
>
> Please review this at https://codereview.appspot.com/66510044/
>
> Affected files (+2, -1 lines):
>   M src/cmd/gc/reflect.c
>
>
> Index: src/cmd/gc/reflect.c
> ===================================================================
> --- a/src/cmd/gc/reflect.c
> +++ b/src/cmd/gc/reflect.c
> @@ -1127,7 +1127,8 @@
>                                         ot = dgopkgpath(s, ot,
> t1->sym->pkg);
>                         } else {
>                                 ot = dgostringptr(s, ot, nil);
> -                               if(t1->type->sym != S &&
> t1->type->sym->pkg == builtinpkg)
> +                               if(t1->type->sym != S &&
> +                                  (t1->type->sym->pkg == builtinpkg ||
> !exportname(t1->type->sym->name)))
>                                         ot = dgopkgpath(s, ot, localpkg);
>                                 else
>                                         ot = dgostringptr(s, ot, nil);
>
>
> --
> 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/groups/opt_out.
>

[gri, 2014-02-20 18:22:19]

yes, please add a test case


On Thu, Feb 20, 2014 at 10:19 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> No new test?
>
>
>
> On Thu, Feb 20, 2014 at 10:17 AM, <cmang@golang.org> wrote:
>
>> Reviewers: gri, rsc,
>>
>> Message:
>> Hello gri@golang.org, rsc@golang.org (cc:
>> golang-codereviews@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go
>>
>>
>> Description:
>> cmd/gc: make embedded, unexported fields read-only.
>>
>> Fixes issue 7363.
>>
>> Please review this at https://codereview.appspot.com/66510044/
>>
>> Affected files (+2, -1 lines):
>>   M src/cmd/gc/reflect.c
>>
>>
>> Index: src/cmd/gc/reflect.c
>> ===================================================================
>> --- a/src/cmd/gc/reflect.c
>> +++ b/src/cmd/gc/reflect.c
>> @@ -1127,7 +1127,8 @@
>>                                         ot = dgopkgpath(s, ot,
>> t1->sym->pkg);
>>                         } else {
>>                                 ot = dgostringptr(s, ot, nil);
>> -                               if(t1->type->sym != S &&
>> t1->type->sym->pkg == builtinpkg)
>> +                               if(t1->type->sym != S &&
>> +                                  (t1->type->sym->pkg == builtinpkg ||
>> !exportname(t1->type->sym->name)))
>>                                         ot = dgopkgpath(s, ot, localpkg);
>>                                 else
>>                                         ot = dgostringptr(s, ot, nil);
>>
>>
>>
>> --
>> 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/groups/opt_out.
>>
>
>

[cmang, 2014-02-20 18:28:55]

On 2014/02/20 18:22:19, gri wrote:
> yes, please add a test case
> 
> 
> On Thu, Feb 20, 2014 at 10:19 AM, Brad Fitzpatrick
<mailto:bradfitz@golang.org>wrote:
> 
> > No new test?
> >
> >
> >
> > On Thu, Feb 20, 2014 at 10:17 AM, <mailto:cmang@golang.org> wrote:
> >
> >> Reviewers: gri, rsc,
> >>
> >> Message:
> >> Hello mailto:gri@golang.org, mailto:rsc@golang.org (cc:
> >> mailto:golang-codereviews@googlegroups.com),
> >>
> >> I'd like you to review this change to
> >> https://code.google.com/p/go
> >>
> >>
> >> Description:
> >> cmd/gc: make embedded, unexported fields read-only.
> >>
> >> Fixes issue 7363.
> >>
> >> Please review this at https://codereview.appspot.com/66510044/
> >>
> >> Affected files (+2, -1 lines):
> >>   M src/cmd/gc/reflect.c
> >>
> >>
> >> Index: src/cmd/gc/reflect.c
> >> ===================================================================
> >> --- a/src/cmd/gc/reflect.c
> >> +++ b/src/cmd/gc/reflect.c
> >> @@ -1127,7 +1127,8 @@
> >>                                         ot = dgopkgpath(s, ot,
> >> t1->sym->pkg);
> >>                         } else {
> >>                                 ot = dgostringptr(s, ot, nil);
> >> -                               if(t1->type->sym != S &&
> >> t1->type->sym->pkg == builtinpkg)
> >> +                               if(t1->type->sym != S &&
> >> +                                  (t1->type->sym->pkg == builtinpkg ||
> >> !exportname(t1->type->sym->name)))
> >>                                         ot = dgopkgpath(s, ot, localpkg);
> >>                                 else
> >>                                         ot = dgostringptr(s, ot, nil);
> >>
> >>
> >>
> >> --
> >> 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 mailto:golang-codereviews+unsubscribe@googlegroups.com.
> >> For more options, visit https://groups.google.com/groups/opt_out.
> >>
> >
> >

Sorry, I thought it was being tracked. PTAL.

[gri, 2014-02-20 18:52:24]

LGTM

https://codereview.appspot.com/66510044/diff/60001/test/fixedbugs/issue7363.go
File test/fixedbugs/issue7363.go (right):

https://codereview.appspot.com/66510044/diff/60001/test/fixedbugs/issue7363.g...
test/fixedbugs/issue7363.go:7: // issue 7363: CanSet returns true for unexported
embedded struct fields.
s/returns true/must return false/

[cmang, 2014-02-20 18:59:33]

https://codereview.appspot.com/66510044/diff/60001/test/fixedbugs/issue7363.go
File test/fixedbugs/issue7363.go (right):

https://codereview.appspot.com/66510044/diff/60001/test/fixedbugs/issue7363.g...
test/fixedbugs/issue7363.go:7: // issue 7363: CanSet returns true for unexported
embedded struct fields.
On 2014/02/20 18:52:25, gri wrote:
> s/returns true/must return false/

Done.

[cmang, 2014-02-20 19:33:03]

*** Submitted as https://code.google.com/p/go/source/detail?r=6c0339d94123 ***

cmd/gc: make embedded, unexported fields read-only.

Fixes issue 7363.

LGTM=gri
R=gri, rsc, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/66510044