code review 7312093: go/types: Pkg *Package field for all objects

go/types: Pkg *Package field for all objects

The field is nil for predeclared (universe)
objects and parameter/result variables.

[gri, 2013-02-13 04:50:17]

Hello adonovan@google.com (cc: golang-dev@googlegroups.com),

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

[adonovan, 2013-02-13 04:57:57]

LGTM.  This is all very straightforward.

https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go
File src/pkg/go/types/objects.go (right):

https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go#...
src/pkg/go/types/objects.go:17: GetPkg() *Package
Under what circumstances is it nil?

I would expect that for function-local 'const', 'var' and 'type' decls and for
'func' literals it would be nil.

https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go#...
src/pkg/go/types/objects.go:56: type Var struct {
I wonder if it's worth defining a different type for the elements of a
types.Signature, since half the space of a Var is unused in that case.

[gri, 2013-02-13 05:04:04]

Let's discuss this tomorrow in detail.
- gri

https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go
File src/pkg/go/types/objects.go (right):

https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go#...
src/pkg/go/types/objects.go:17: GetPkg() *Package
On 2013/02/13 04:57:58, adonovan wrote:
> Under what circumstances is it nil?
> 
> I would expect that for function-local 'const', 'var' and 'type' decls and for
> 'func' literals it would be nil.

Right now it should only be nil for objects that are in no package (universe),
and parameters/results.

We can define what we want - for instance we could only set it for top-level
(package-level) objects. That would make a lot of sense. The invariant would be
that for such an object obj we can always find it in obj.Pkg.Scope . Maybe
that's the right definition.

https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go#...
src/pkg/go/types/objects.go:56: type Var struct {
On 2013/02/13 04:57:58, adonovan wrote:
> I wonder if it's worth defining a different type for the elements of a
> types.Signature, since half the space of a Var is unused in that case.

Yes, I have been wondering, too. It might clean up a few things.

[adonovan, 2013-02-13 05:06:35]

On 13 February 2013 00:04, <gri@golang.org> wrote:

> Let's discuss this tomorrow in detail.
> - gri
>
>
>
> https://codereview.appspot.**com/7312093/diff/6001/src/pkg/**
>
go/types/objects.go<https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go>
> File src/pkg/go/types/objects.go (right):
>
> https://codereview.appspot.**com/7312093/diff/6001/src/pkg/**
>
go/types/objects.go#newcode17<https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go#newcode17>
> src/pkg/go/types/objects.go:**17: GetPkg() *Package
> On 2013/02/13 04:57:58, adonovan wrote:
>
>> Under what circumstances is it nil?
>>
>
>  I would expect that for function-local 'const', 'var' and 'type' decls
>>
> and for
>
>> 'func' literals it would be nil.
>>
>
> Right now it should only be nil for objects that are in no package
> (universe), and parameters/results.
>
> We can define what we want - for instance we could only set it for
> top-level (package-level) objects. That would make a lot of sense. The
> invariant would be that for such an object obj we can always find it in
> obj.Pkg.Scope . Maybe that's the right definition.


Yes, that's what makes most sense to me.  What you have here is progress
though.



>
>
> https://codereview.appspot.**com/7312093/diff/6001/src/pkg/**
>
go/types/objects.go#newcode56<https://codereview.appspot.com/7312093/diff/6001/src/pkg/go/types/objects.go#newcode56>
> src/pkg/go/types/objects.go:**56: type Var struct {
> On 2013/02/13 04:57:58, adonovan wrote:
>
>> I wonder if it's worth defining a different type for the elements of a
>> types.Signature, since half the space of a Var is unused in that case.
>>
>
> Yes, I have been wondering, too. It might clean up a few things.
>
>
https://codereview.appspot.**com/7312093/<https://codereview.appspot.com/7312...
>

[gri, 2013-02-18 22:41:06]

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

go/types: Pkg *Package field for all objects

The field is nil for predeclared (universe)
objects and parameter/result variables.

R=adonovan
CC=golang-dev
https://codereview.appspot.com/7312093