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

cmd/compile: incorrect pluralization in error message #45369

Closed
yurivish opened this issue Apr 3, 2021 · 5 comments
Closed

cmd/compile: incorrect pluralization in error message #45369

yurivish opened this issue Apr 3, 2021 · 5 comments

Comments

@yurivish
Copy link

@yurivish yurivish commented Apr 3, 2021

Go 1.16 produces incorrectly pluralized error message like this one:

assignment mismatch: 2 variables but r.Form.Get returns 1 values

The error is generated by this line:

base.Errorf("assignment mismatch: %d variables but %d values", len(names), len(exprs))

which can be fixed by using a plural function, e.g.:

base.ErrorfAt(stmt.Pos(), "assignment mismatch: %d variable%s but %v returns %d value%s", len(lhs), plural(len(lhs)), r.X, cr, plural(cr))

@ALTree ALTree changed the title cmd/compile: Typo in error message cmd/compile: incorrect pluralization in error message Apr 5, 2021
@ALTree ALTree added the NeedsFix label Apr 5, 2021
@ALTree ALTree added this to the Unplanned milestone Apr 5, 2021
@ALTree
Copy link
Member

@ALTree ALTree commented Apr 5, 2021

Go 1.16 produces incorrectly pluralized error message like this one:

Please post a program that can be used to reproduce the issue.

This seem another case of #31099, which is fixed now on tip.

Loading

@yurivish
Copy link
Author

@yurivish yurivish commented Apr 5, 2021

@ALTree

Here's an example program (https://play.golang.org/p/pdxWzlrXTnj):

package main

func main() {
	a, b := 1 + 1
}

Apologies for not searching before filing this issue and thanks for the reference. Good to know it'll be fixed in the upcoming release!

Loading

@ALTree
Copy link
Member

@ALTree ALTree commented Apr 5, 2021

Thanks for the reproducer. I can confirm that the issue is fixed on your example on tip:

$ gotip version
go version devel go1.17-7bfd681c2f Sun Apr 4 12:04:33 2021 +0000 linux/amd64

$ gotip build test.go

./test.go:4:7: assignment mismatch: 2 variables but 1 value

It seems that the error message is printed from

cmd/compile/internal/typecheck/stmt.go (line 187)

so we no longer execute the line with the bad pluralization in noder.go that you linked above. Still, that line does not pluralize correctly, and if some codepath ends up executing it we'll have a bad pluralization. Nothing urgent, but still.

cc @mdempsky.

Loading

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 5, 2021

Those code paths won't be used anymore once we switch to the new typechecker, which is expected for Go 1.18 with generics and likely even in Go 1.17.

Loading

@ALTree
Copy link
Member

@ALTree ALTree commented Apr 5, 2021

Thanks! Then we can close this, since no further action is needed to fix the issue, and on tip the correct message is already printed.

Loading

@ALTree ALTree closed this Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@mdempsky @yurivish @ALTree and others