code review 6594054: /exp/types/staging: expression and statement type checking

/exp/types/staging: expression and statement type checking

Still lots of pieces missing, but basic framework working.
Lots of tests.

[gri, 2012-10-01 22:46:11]

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

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

[rsc, 2012-10-06 22:43:20]

LGTM

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
File src/pkg/exp/types/staging/expr.go (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:79: // TODO(gri) AST may be invalid - check
Unquote result
Same above?

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:116: check.invalidOp(x.pos(), "operator %s not
defined for %s", op, x)
This is a vague error message for the specific case of receiving from a
send-only channel. I would suggest giving a more specific error, either by
leaving that for a higher-level pass, or by handling it specially here. <-c is
certainly defined in that case, it's just not allowed.

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:393: 
?
Maybe already handled above?

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:713: if i, ok := x.val.(int64); ok && i ==
int64(int(i)) {
I think you need an explicit conversion to integer here. 
var x [1.0]int
appears to be valid.

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/st...
File src/pkg/exp/types/staging/stmt.go (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/st...
src/pkg/exp/types/staging/stmt.go:16: check.errorf(x.pos(), "%d-valued
expression %s used as single value", len(t.list), x)
below you use argument count mismatch: %d = %d for what looks like the same
case?

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/te...
File src/pkg/exp/types/staging/testdata/decls0.src (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/te...
src/pkg/exp/types/staging/testdata/decls0.src:103: A1 [10.0]int
And yet this test case appears to work so perhaps I misunderstand the code.

[gri, 2012-10-07 23:27:20]

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
File src/pkg/exp/types/staging/expr.go (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:79: // TODO(gri) AST may be invalid - check
Unquote result
On 2012/10/06 22:43:20, rsc wrote:
> Same above?

Replaced in both places with a function extracting and checking the field tag.

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:393: 
On 2012/10/06 22:43:20, rsc wrote:
> ?
> Maybe already handled above?

dregs. removed.

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:713: if i, ok := x.val.(int64); ok && i ==
int64(int(i)) {
On 2012/10/06 22:43:20, rsc wrote:
> I think you need an explicit conversion to integer here. 
> var x [1.0]int
> appears to be valid.

Constants are normalized - so 1.0 will be represented as an int64. This is
correct and there's a test for it.

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/st...
File src/pkg/exp/types/staging/stmt.go (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/st...
src/pkg/exp/types/staging/stmt.go:16: check.errorf(x.pos(), "%d-valued
expression %s used as single value", len(t.list), x)
On 2012/10/06 22:43:20, rsc wrote:
> below you use argument count mismatch: %d = %d for what looks like the same
> case?

I have copied the gc error message style here ("multiple-value f() in
single-value context"). Added a TODO for later cleanup/consolidation of error
messages.

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/te...
File src/pkg/exp/types/staging/testdata/decls0.src (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/te...
src/pkg/exp/types/staging/testdata/decls0.src:103: A1 [10.0]int
On 2012/10/06 22:43:20, rsc wrote:
> And yet this test case appears to work so perhaps I misunderstand the code.

Yes. See comment before.

[gri, 2012-10-08 00:19:45]

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
File src/pkg/exp/types/staging/expr.go (right):

https://codereview.appspot.com/6594054/diff/7001/src/pkg/exp/types/staging/ex...
src/pkg/exp/types/staging/expr.go:116: check.invalidOp(x.pos(), "operator %s not
defined for %s", op, x)
On 2012/10/06 22:43:20, rsc wrote:
> This is a vague error message for the specific case of receiving from a
> send-only channel. I would suggest giving a more specific error, either by
> leaving that for a higher-level pass, or by handling it specially here. <-c is
> certainly defined in that case, it's just not allowed.
> 

Added TODO.

[gri, 2012-10-08 01:01:52]

*** Submitted as http://code.google.com/p/go/source/detail?r=0a11e5a60d9c ***

/exp/types/staging: expression and statement type checking

Still lots of pieces missing, but basic framework working.
Lots of tests.

R=rsc
CC=golang-dev
http://codereview.appspot.com/6594054