Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

go/types, types2: remove discrepancy between type-checker and compiler error messages #55326

Closed
griesemer opened this issue Sep 21, 2022 · 26 comments
Closed

go/types, types2: remove discrepancy between type-checker and compiler error messages #55326

griesemer opened this issue Sep 21, 2022 · 26 comments
Assignees
@griesemer
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.20

Comments

@griesemer
Copy link
Contributor

griesemer commented Sep 21, 2022 •
edited

For historic reasons, some errors are report with different messages by the compiler and type checkers. For instance, the compiler reports "undefined: x" while the type checkers report "undeclared name: x" for an unknown identifier x.

types2 has a configuration flag (Config.UseCompilerErrorMessages) that is set if types2 is used by the compiler; go/types has an internal constant useCompilerErrorMessages (set to false) for symmetry of code.

We want to remove the discrepancy and standardize on just one error message, and the eliminate the Config.UseCompilerErrorMessages (types2) and useCompilerErrorMessages (go/types) flags altogether.

At this point there are 16 cases where error messages differ:

  Source example Compiler go/types, types2
       
1 var _ = x undefined: x undeclared name: x
2 var o = math.Omega undefined: math.Omega Omega not declared by package math
3 type T struct{}; func (T) m() {} // twice T.m redeclared in this block method m already declared for type T struct{}
4 import "math" imported and not used: "math" "math" imported but not used
5 import foo "math" imported and not used: "math" as foo "math" imported but not used as foo
6 s = x cannot use x (variable of type int) as type string in assignment cannot use x (variable of type int) as string value in assignment
7 var a, b = f() assignment mismatch: 2 variables but f returns 3 values cannot initialize 2 variables with 3 values
8 a, b = f() assignment mismatch: 2 variables but f returns 3 values cannot assign 3 values to 2 variables
9 _ = int(s) cannot convert s (variable of type string) to type int cannot convert s (variable of type string) to int
10 type T struct{ T } invalid recursive type T illegal cycle in declaration of T
11 _ = 1.0 << s invalid operation: 1.0 << s (shift of type float64) invalid operation: shifted operand 1.0 (type float64) must be integer
12 _ = 1 == true invalid operation: 1 == true (mismatched types ...) invalid operation: cannot compare 1 == true (mismatched types ...)
13 type S struct{}; _ = S{f: 0} unknown field 'f' in struct literal of type S unknown field f in struct literal
14 func f() (int, int); var x = f() multiple-value f() (value of type (int, int)) in single-value context 2-valued f() (value of type (int, int)) where single value is expected
15 var a, b = b, a initialization loop for a initialization cycle for a
16 var x interface{ m() } = 0 cannot use 0 (constant of type int) as ...: ...(missing m method) cannot use 0 (constant of type int) as ...: ...(missing method m)
@griesemer griesemer added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 21, 2022
@griesemer griesemer added this to the Go1.20 milestone Sep 21, 2022
@griesemer griesemer self-assigned this Sep 21, 2022
@gopherbot
Copy link

Change https://go.dev/cl/432555 mentions this issue: go/types, types2: report "undefined: x" instead of "undeclared named: x"

@gopherbot
Copy link

Change https://go.dev/cl/432556 mentions this issue: go/types, types2: report "undefined: p.x" instead of "x not declared by package p"

@gopherbot
Copy link

Change https://go.dev/cl/432557 mentions this issue: cmd/compile: use "pkg" imported [as X] but not used error message

@gopherbot
Copy link

Change https://go.dev/cl/432558 mentions this issue: cmd/compile: consistently use "but not used" instead of "and not used"

@gopherbot
Copy link

Change https://go.dev/cl/432559 mentions this issue: cmd/compile: handle go.mod error msg reference in noder, not type checker

@gopherbot
Copy link

Change https://go.dev/cl/432615 mentions this issue: cmd/compile: use "init... cycle" rather then "init... loop" in error messages

@gopherbot
Copy link

Change https://go.dev/cl/432635 mentions this issue: go/types,types2: using "missing method m" instead of "missing m method"

@jacobishao jacobishao mentioned this issue Sep 22, 2022
Closed
@gopherbot
Copy link

Change https://go.dev/cl/432655 mentions this issue: use "cannot convert %s to type %s" instead of "cannot convert %s to %s"

@gopherbot
Copy link

Change https://go.dev/cl/432636 mentions this issue: go/types,types2: unify error message to "invalid operation: cannot compare..."

@gopherbot
Copy link

Change https://go.dev/cl/432637 mentions this issue: go/types,types2: unify error message to "invalid operation: shifted operand"

@gopherbot
Copy link

Change https://go.dev/cl/432638 mentions this issue: go/types: unify error message to "invalid recursive type"

@gopherbot
Copy link

Change https://go.dev/cl/432639 mentions this issue: go/types,types2: unify error message to "cannot convert .. to type ..."

@gopherbot
Copy link

Change https://go.dev/cl/432640 mentions this issue: go/types,types2: unify error message to "assignment mismatch: ..."

@gopherbot
Copy link

Change https://go.dev/cl/432641 mentions this issue: go/types,types2: unify error message to "cannot use ... as ... value in assignment"

@gopherbot
Copy link

Change https://go.dev/cl/433055 mentions this issue: go/types, types2: use "invalid recursive type" instead of "illegal cycle" in error messages

@gopherbot
Copy link

Change https://go.dev/cl/433276 mentions this issue: go/types, types2: uniformly use "cannot convert X to type T"

gopherbot pushed a commit that referenced this issue Sep 23, 2022
@griesemer @gopherbot
cmd/compile: handle go.mod error msg reference in noder, not type che…
5d213a3 
…cker

Currently, for version errors, types2 adds the helpful hint

(-lang was set to go1.xx; check go.mod)

where 1.xx is the respective language version, to the error message.
This requires that the type checker knows that it was invoked by the
compiler, which is done through the Config.CompilerErrorMessages flag.

This change looks for version errors being returned by the type checker
and then adds the hint at that point, external to the type checker.
This removes a dependency on the Config.CompilerErrorMessages. Once
we have removed all dependencies on Config.CompilerErrorMessages we
can remove it.

For #55326.

Change-Id: I1f9b2e472c49fe785a2075e26c4b3d9b8fcdbf4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/432559
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit that referenced this issue Sep 23, 2022
@griesemer @gopherbot
cmd/compile: use "init... cycle" instead of "init... loop" in error m…
c58bfee 
…essages

For #55326.

Change-Id: Ia3c1124305986dcd49ac769e700055b263cfbd59
Reviewed-on: https://go-review.googlesource.com/c/go/+/432615
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
gopherbot pushed a commit that referenced this issue Sep 24, 2022
@cuiweixie @gopherbot
cmd/compile: use "missing method m" instead of "missing m method"
336ce96 
For #55326

Change-Id: I3d0ff7f820f7b2009d1b226abf701b2337fe8cbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/432635
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: xie cui <523516579@qq.com>
Reviewed-by: Robert Griesemer <gri@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/434735 mentions this issue: go/types, types2: use "multiple-value" instead "n-valued" in error messages

@gopherbot
Copy link

Change https://go.dev/cl/434815 mentions this issue: cmd/compile: use "unknown field f in struct literal of type S" in error messages

gopherbot pushed a commit that referenced this issue Sep 26, 2022
@griesemer
go/types, types2: use "invalid recursive type" instead of "illegal cy…
22f4471 
…cle" in error messages

This matches long-standing compiler behavior.

For #55326.

Change-Id: Ic5aa0dfb08d035f2c33532cc463c73a55cc020a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/433055
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit that referenced this issue Sep 26, 2022
@griesemer
go/types, types2: report "undefined: x" instead of "undeclared name: x"
2da95e0 
This matches the compiler's long-standing behavior.

For #55326.

Change-Id: I90696a11f0b7d1f4be95a4b9a6f01844df2a2347
Reviewed-on: https://go-review.googlesource.com/c/go/+/432555
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Sep 26, 2022
@griesemer
go/types, types2: report "undefined: p.x" instead of "x not declared …
17f83e3 
…by package p"

This matches the compiler's long-standing behavior.

For #55326.

Change-Id: Icd946b031b1b6e65498fb52bceb4a53807732463
Reviewed-on: https://go-review.googlesource.com/c/go/+/432556
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/435016 mentions this issue: cmd/compile: use "method T.m already declared" for method redeclaration errors

@gopherbot
Copy link

Change https://go.dev/cl/435315 mentions this issue: cmd/gorelease: update expected errors (fix build)

gopherbot pushed a commit to golang/exp that referenced this issue Sep 27, 2022
@griesemer @gopherbot
cmd/gorelease: update expected errors (fix build)
c76eaa3 
CL 432555 changed the go/types error message for undeclared names
from "undeclared name: X" to "undefined: X" to match the compiler.
Update the test files.

For golang/go#55326.

Change-Id: I175705ef1aa357f6578cd05025a35e52520e523c
Reviewed-on: https://go-review.googlesource.com/c/exp/+/435315
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Sep 27, 2022
@griesemer @gopherbot
go/types, types2: use "and not used" instead of "but not used" in err…
4360fd8 
…or messages

This matches longstanding compiler behavior.

Also, for unused packages, report:

`"pkg" imported and not used`
`"pkg" imported as X and not used`

This matches the other `X declared and not used` errors.

For #55326.

Change-Id: Ie71cf662fb5f4648449c64fc51bede298a1bdcbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/432557
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
gopherbot pushed a commit that referenced this issue Sep 27, 2022
@griesemer @gopherbot
go/types, types2: uniformly use "cannot convert X to type T"
605148c 
This matches current compiler behavior.

For #55326.

Change-Id: I660bd15f13a8d9eb00fafa937f8261e664b0e065
Reviewed-on: https://go-review.googlesource.com/c/go/+/433276
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
gopherbot pushed a commit that referenced this issue Sep 27, 2022
@griesemer @gopherbot
go/types, types2: use "multiple-value" instead "n-valued" in error me…
3b5188e 
…ssages

This matches current compiler behavior.

For #55326.

Change-Id: I9ebe2914323072b5454fb9af2d15c9dd2d711bad
Reviewed-on: https://go-review.googlesource.com/c/go/+/434735
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
gopherbot pushed a commit that referenced this issue Sep 27, 2022
@griesemer
go/types, types2: use "unknown field f in struct literal of type S" i…
b16501c 
…n error messages

This is a compromise of the error reported by the compiler (quotes
around field name removed) and the error reported by the type checkers
(added mention of struct type).

For #55326.

Change-Id: Iac4fb5c717f17c6713e90d327d39e68d3be40074
Reviewed-on: https://go-review.googlesource.com/c/go/+/434815
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
gopherbot pushed a commit that referenced this issue Sep 27, 2022
@griesemer
cmd/compile: use "method T.m already declared" for method redeclarati…
7398c3c 
…on errors

Compromise between old compiler error "T.m redeclared in this block"
(where the "in this block" is not particularly helpful) and the old
type-checker error "method m already declared for type T ...".
In the case where we have position information for the original
declaration, the error message is "method T.m already declared at
<position>". The new message is both shorter and more precise.

For #55326.

Change-Id: Id4a7f326fe631b11db9e8030eccb417c72d6c7db
Reviewed-on: https://go-review.googlesource.com/c/go/+/435016
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/435555 mentions this issue: go/types, types2: use 2nd operand position for comparison type mismatch errors

gopherbot pushed a commit that referenced this issue Sep 27, 2022
@griesemer @gopherbot
go/types, types2: use 2nd operand position for comparison type mismat…
b2137e7 
…ch errors

When a comparison is invalid due to mismatched types, we only know
when we see the 2nd operand; so use that operand's position for the
error message. This matches compiler behavior.

For #55326.

Change-Id: I79450756bbdd2b4bb90ed4e960a451be0197b186
Reviewed-on: https://go-review.googlesource.com/c/go/+/435555
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/435618 mentions this issue: go/types, types2: use "invalid operation: x rel y (cause)" for compare error messages

gopherbot pushed a commit that referenced this issue Sep 28, 2022
@griesemer @gopherbot
go/types, types2: use "invalid operation: x rel y (cause)" for compar…
dbe56ff 
…ison error messages

Matches compiler behavior and is consistent with what we do with other
binary operations.

While at it, also use parentheses rather than a colon for a couple of
errors caused by not having a core type.

For #55326.

Change-Id: I0a5cec1a31ffda98d363e5528791965a1ccb5842
Reviewed-on: https://go-review.googlesource.com/c/go/+/435618
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/436175 mentions this issue: go/types, types2: use "assignment mismatch: x variables but y values" error message

gopherbot pushed a commit that referenced this issue Sep 28, 2022
@griesemer @gopherbot
go/types, types2: use "assignment mismatch: x variables but y values"…
435652b 
… error message

This matches current compiler behavior.

For #55326.

Change-Id: I7197cf4ce21e614291a1a2e1048dd78d0a232b64
Reviewed-on: https://go-review.googlesource.com/c/go/+/436175
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/436176 mentions this issue: cmd/compile: use "cannot use %s as %s value in %s: %s" error message

@gopherbot
Copy link

Change https://go.dev/cl/436177 mentions this issue: cmd/compile: use "shifted operand %s (type %s) must be integer" for some shift errors

@gopherbot
Copy link

Change https://go.dev/cl/435416 mentions this issue: go/types, types2: remove (C/c)ompilerErrorMessages flag - not needed anymore

gopherbot pushed a commit that referenced this issue Sep 28, 2022
@griesemer @gopherbot
cmd/compile: use "cannot use %s as %s value in %s: %s" error message
7997e5f 
This is close to what the compiler used to say, except now we say
"as T value" rather than "as type T" which is closer to the truth
(we cannot use a value as a type, after all). Also, place the primary
error and the explanation (cause) on a single line.

Make respective (single line) adjustment to the matching "cannot
convert" error.

Adjust various tests.

For #55326.

Change-Id: Ib646cf906b11f4129b7ed0c38cf16471f9266b88
Reviewed-on: https://go-review.googlesource.com/c/go/+/436176
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Sep 28, 2022
@griesemer @gopherbot
cmd/compile: use "shifted operand %s (type %s) must be integer" for s…
8c29881 
…ome shift errors

This matches what go/types and types2 report and it also matches
the compiler errors reported for some related shift problems.

For #55326.

Change-Id: Iee40e8d988d5a7f9ff2c49f019884d02485c9fdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/436177
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@griesemer griesemer

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
2 participants
@griesemer @gopherbot