-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/compile: better error messages from types2 (Go 1.18) for unexported fields/methods #49736
Comments
For issue 18419, the error message is longer in 1.18 but provide equal if not more information. |
cc: @findleyr |
I don't think that's entirely true. |
Yes, I agree on 18419 - the message is factually incorrect. In both cases, essentially types2 is not noting that the real problem is that the available field/method is not exported, so can't be used. (I guess in issue 18419, you could argue that the more likely error the user is making is not calling the upper-case |
Change https://golang.org/cl/378177 mentions this issue: |
I've mailed a fix for this for types2 per CL https://golang.org/cl/378177, please take a look. |
Moving to 1.19. There's too many parts that may need to be adjusted. Also, what we have now is ok. |
Still good to fix, but not urgent. Moving to 1.20. |
Too late for 1.21. Moving forward. |
Too late for 1.22. Moving forward. |
This issue is currently labeled as early-in-cycle for Go 1.23. |
Change https://go.dev/cl/560055 mentions this issue: |
This CL improves the error messages reported when a field or method name is used that doesn't exist. It brings the error messges on par (or better) with the respective errors reported before Go 1.18 (i.e. before switching to the new type checker): Make case distinctions based on whether a field/method is exported and how it is spelled. Factor out that logic into a new function (lookupError) in a new file (errsupport.go), which is generated for go/types. Use lookupError when reporting selector lookup errors and missing struct field keys. Add a comprehensive set of tests (lookup2.go) and spot tests for the two cases brought up by the issue at hand. Adjusted existing tests as needed. Fixes golang#49736. Change-Id: I2f439948dcd12f9bd1a258367862d8ff96e32305 Reviewed-on: https://go-review.googlesource.com/c/go/+/560055 Run-TryBot: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
The error messages from types2 for Go 1.18, I think, as less helpful than the Go 1.17 (-G=0) messages in the case of unexported fields and members. This is shown for the two existing bug test cases in go/test/fixedbugs, issue18419.go and issue31053.go.
We have disabled these tests in run.go because of different error message output for Go 1.17 and Go.18, and the difference is because Go 1.18 is not helpfully indicating the problem is due to an unexported field/method, but is instead indicating the field/method is unknown or doesn't exist.
(I'm going through a bunch of the other disabled tests, which are mostly due to largely equivalent error messages, just slight different words or a few extra, related errors. One area that I will work on fixing is missing errors because of improper compiler directives (//go:...), which are mostly ignored by types2.)
You can enable the test for each of these issues by commenting out the specified issue in types2Failures array in test/run.go.
For issue 18419, the Go 1.17 error message is:
whereas for Go 1.18 it is:
For issue 31053, there are many error messages that match, but the two relevant ones that differ are for Go 1.17:
and for Go 1.18:
If it makes sense to fix, I'm sure this is fine to fix either before or after the beta release.
@griesemer @findleyr
The text was updated successfully, but these errors were encountered: