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: confusing compiler errors for bad method name on named struct literal #38745
Comments
I agree that instead of But I don't see a problem with the two error messages here: There are indeed two different errors: Maybe in the actual code, the actual function that was supposed to be called here has two result values, but the compiler cannot know that. In general, in a case such as this, we do want both errors. |
I don't think we should change I'd support changing it to something like I think one error would make sense though. We only emit one error (
But either way, we should probably be consistent. |
Fair enough. |
I would argue that the compiler can't know that. Funcs can have multiple results, so until the identifier is defined, it can't know that there's a result count mismatch. It would be like reporting errors for For: func g() (int, int) { return h() } where
instead of:
For: var x, y = f() where
instead of:
It seems like all these scenarios should result in the same error set. It looks like the compiler is trying to show the actual problematic syntax along with the type error explanation. func f(...int) T { return T{} }
func F() (*R, error) {
return f(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10).M()
} results in:
To be consistent with func calls, at least, it should be |
Agreed we should try to be more consistent here between function calls and composite literals, but I think the preferable direction would be towards reporting Ideally, we'd have some clever way of knowing what context we can reasonably trim, and a smart threshold for when trimming is appropriate (e.g., no point trimming |
Change https://golang.org/cl/253677 mentions this issue: |
Rob and Minux don't like I also notice that (To follow gofmt style though, we should use |
Change https://golang.org/cl/253678 mentions this issue: |
This CL changes "T literal.M" error message to "T{...}.M". It's clearer expression and focusing user on actual issue. Updates #38745 Change-Id: I84b455a86742f37e0bde5bf390aa02984eecc3c9 Reviewed-on: https://go-review.googlesource.com/c/go/+/253677 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://play.golang.org/p/ms1UAARp88o
What did you expect to see?
What did you see instead?
It's confusing that the actionable error is listed second. The first error seems to be happening because it's assuming that M (which the compiler should know that it knows nothing about) returns only one result, but the underlying result of that is the second error. Only the second error should be reported.
Also,
T literal.M
is confusing syntax/wording.T.M
seems clear enough.The text was updated successfully, but these errors were encountered: