code review 7473043: go/types: cleanup of assignment checks

go/types: cleanup of assignment checks

Also:
- cleaner handling of constants w/ unknown value
- removed several TODOs

[gri, 2013-03-06 20:45:57]

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-03-06 22:11:08]

LGTM

I confess I didn't follow all the details of the assign1to1 refactoring.

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/check.go
File src/pkg/go/types/check.go (right):

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/check.go#n...
src/pkg/go/types/check.go:209: // value of the constant (e.g.m, in: "const x =
float32("foo")" we
rogue 'm'?

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/const.go
File src/pkg/go/types/const.go (right):

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/const.go#n...
src/pkg/go/types/const.go:22: // invalid  ->  nil (i.e., we don't know the
constant value)
If it's an invariant that this never happens in well-typed programs, it would be
good to state it.

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

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/objects.go...
src/pkg/go/types/objects.go:41: Val  interface{} // nil means unknown constant
value
How about: "nil means constant value unknown due to error"?

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/stmt.go
File src/pkg/go/types/stmt.go (right):

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/stmt.go#ne...
src/pkg/go/types/stmt.go:44: var obj Object
How about moving LL44-60 down, immediately after the assignment to obj?  This
has better reading locality (and avoids some unnecessary deferred closures in
the error and "_ = x" cases).

[gri, 2013-03-06 23:45:55]

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/check.go
File src/pkg/go/types/check.go (right):

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/check.go#n...
src/pkg/go/types/check.go:209: // value of the constant (e.g.m, in: "const x =
float32("foo")" we
On 2013/03/06 22:11:09, adonovan wrote:
> rogue 'm'?

Done.

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/const.go
File src/pkg/go/types/const.go (right):

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/const.go#n...
src/pkg/go/types/const.go:22: // invalid  ->  nil (i.e., we don't know the
constant value)
On 2013/03/06 22:11:09, adonovan wrote:
> If it's an invariant that this never happens in well-typed programs, it would
be
> good to state it.

Done.

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

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/objects.go...
src/pkg/go/types/objects.go:41: Val  interface{} // nil means unknown constant
value
On 2013/03/06 22:11:09, adonovan wrote:
> How about: "nil means constant value unknown due to error"? 

Done.

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/stmt.go
File src/pkg/go/types/stmt.go (right):

https://codereview.appspot.com/7473043/diff/22001/src/pkg/go/types/stmt.go#ne...
src/pkg/go/types/stmt.go:44: var obj Object
On 2013/03/06 22:11:09, adonovan wrote:
> How about moving LL44-60 down, immediately after the assignment to obj?  This
> has better reading locality (and avoids some unnecessary deferred closures in
> the error and "_ = x" cases).

Done.

[gri, 2013-03-07 00:14:30]

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

go/types: cleanup of assignment checks

Also:
- cleaner handling of constants w/ unknown value
- removed several TODOs

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