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: confusing (but accurate) error message for function with variadic arguments if argument is not the last #28450

Closed
loren-osborn opened this issue Oct 28, 2018 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@loren-osborn
Copy link

loren-osborn commented Oct 28, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.10 and playground (1.11)

Does this issue reproduce with the latest release?

Yes. Example: https://play.golang.org/p/kE_ICtDSVzU

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

I forgot to include an argument type prior to a final variadic function argument.

Here is an example: https://play.golang.org/p/kE_ICtDSVzU

In this example, bar was intended to be a string, but the compiler had no way of knowing this.

What did you expect to see?

Given the code I was attempting to compile, I would expect an error message to identify a

missing type on argument bar

since, (as the actual error message indicated, only use ... with final parameter in list) a variadic type of args should not be applied to a list of arguments.

What did you see instead?

Error message

can only use ... with final parameter in list

Removing developer attention from bar and directed it toward args ...interface{}

@odeke-em odeke-em changed the title Confusing (but accurate) error message compiling function with variadic arguments cmd/compile: confusing (but accurate) error message for function with variadic arguments if argument is not the last Oct 28, 2018
@odeke-em
Copy link
Member

Thank you for filing this issue @loren-osborn and welcome to the Go project!

@griesemer @mdempsky, what do y'all think of these suggestions?
a)

missing type for argument bar (can only use ... with final parameter in list)

b)

missing type for argument bar

b) is succinct but will likely generate more confusion say if the function signature contains more than one such repeated field e.g.

func Foo(bar, foo, fizz, args ...interface{}) {
}

Hence I am leaning towards suggestion a) which will print out

missing type for argument bar (can only use ... with final parameter in list)
missing type for argument foo (can only use ... with final parameter in list)
missing type for argument fizz (can only use ... with final parameter in list)

or later on, perhaps after the frontend overhaul we can provide a more concise message

missing type for arguments bar, foo, fizz: ... can only be used for the final parameter

@odeke-em odeke-em added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 28, 2018
@mdempsky
Copy link
Member

Thank you for the report.

I agree that the error message can be improved. Certainly, the error's column number information should at least point at the non-final parameter in question, and probably the parameter's name can be incorporated into the error message. I think these would address the raised issue about users misfocusing on args ...interface{} instead of bar.

However, I disagree with the proposed "missing type" wording. In the linked example (https://play.golang.org/p/2EFkTjC3KhA), bar does have a type: ...interface{}. The error is just that this type is invalid, because bar is not the final parameter.

I'd probably lean towards changing the error message to:

cannot use ... with non-final parameter bar

@griesemer
Copy link
Contributor

Not urgent. Good starter project.

@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 29, 2018
@griesemer griesemer added this to the Unplanned milestone Oct 29, 2018
@loren-osborn
Copy link
Author

@mdempsky, I almost agree. The problem is that ...interface{} isn’t a type. The type of args is actually []interface{} imparted by the ... variadic operator. If this operator was defined to only ever apply to one argument, then bar wouldn’t have a type, and the error message I suggested would make sense. I’m not opposed to your suggested error message, I just feel it’s another solution to the same problem.

@gopherbot
Copy link

Change https://golang.org/cl/152417 mentions this issue: cmd/compile: improve error message for non-final variadic parameter

@gopherbot
Copy link

Change https://golang.org/cl/152758 mentions this issue: cmd/compile: avoid multiple errors regarding misuse of ... in signatures

gopherbot pushed a commit that referenced this issue Dec 5, 2018
Follow-up on #28450 (golang.org/cl/152417).

Updates #28450.
Fixes #29107.

Change-Id: Ib4b4fe582c35315a4f71cf6dbc7f7f2f24b37ec1
Reviewed-on: https://go-review.googlesource.com/c/152758
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@golang golang locked and limited conversation to collaborators Dec 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

5 participants