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: change "multiple-value .. in single-value context" error message to be consistent with >1 case #26616

Closed
techtonik opened this issue Jul 26, 2018 · 10 comments

Comments

Projects
None yet
6 participants
@techtonik
Copy link

commented Jul 26, 2018

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

1.10

What did you do?

bar := bufio.NewReader(...)
line := bar.ReadString()

What did you expect to see?

1 of 2 return values of ReadString() is not handled

What did you see instead?

multiple-value bar.ReadString() in single-value context

The expected error text should be more clear for people without a university degree. =)

@meirf

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2018

It looks like 1 vs >1 is a special case. For example, this code:

package main

func main() {
	x, y := foo()
	println(x, y)
}

func foo() (int, int, int) {
	return 0, 0, 0
}

results in "assignment mismatch: 2 variables but 3 values", which is close to what you expect.

@meirf meirf changed the title Simplify "multiple-value bar.ReadString() in single-value context" error message cmd/compile: simplify "multiple-value .. in single-value context" error message Jul 26, 2018

@techtonik

This comment has been minimized.

Copy link
Author

commented Jul 26, 2018

I would still insist on 1 of 3 return values of foo() is not handled.

@agnivade

This comment has been minimized.

Copy link
Member

commented Jul 26, 2018

The error message is the same vein of how we write failure messages in test - Got- %v, Expected- %v. It says - Got 2 variables, but expected 3 values. It clearly mentions what was expected, and what it received.

I am not sure why the single-value context is considered a special case.

/cc @josharian @mdempsky

@andybons andybons added this to the Unplanned milestone Jul 26, 2018

@meirf

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2018

@techtonik:

Regardless of whether the 1 vs >1 special case is changed, it sounds like you prefer an alt error message for the general case. I have a couple concerns with your approach:

I would still insist on 1 of 3 return values of foo() is not handled.

  • "1 of 3 return values": This sounds like an ordering. People might think this means that the first of the three returned values doesn't have a variable to be assigned to but the others do. Whereas "1 variables but 3 values" is just about quantity.
  • "is not handled": The word "handle" is a bit vague. Whereas "assignment mismatch" is more precise.
@mdempsky

This comment has been minimized.

Copy link
Member

commented Jul 27, 2018

I think it's very unlikely we'd adopt the wording 1 of 2 return values of ReadString() is not handled, for the reasons @meirf explained.

I'm sympathetic to changing the error message to assignment mismatch: 1 variable but 2 values though. I think the current error message can be attributed to an artifact of the compiler having OAS as a special case of the more general OAS2 node, rather than being intentionally inconsistent.

If anyone wants to dig into this, look at typecheckas and typecheckas2 in typecheck.go. In particular, Efnstruct (to suppress the "multi-value in single-value context" error) and IsFuncArgStruct (to distinguish a multi-value expression).

@agnivade agnivade changed the title cmd/compile: simplify "multiple-value .. in single-value context" error message cmd/compile: change "multiple-value .. in single-value context" error message to be consistent with >1 case Aug 12, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Aug 12, 2018

Change https://golang.org/cl/129065 mentions this issue: cmd/compile: single value error matches multiple value error

@gopherbot

This comment has been minimized.

Copy link

commented Aug 24, 2018

Change https://golang.org/cl/131280 mentions this issue: cmd/compile: use "N variables but M values" error for OAS

@techtonik

This comment has been minimized.

Copy link
Author

commented Sep 1, 2018

"1 of 3 return values": This sounds like an ordering. People might think this means that the first of the three returned values doesn't have a variable to be assigned to but the others do.

For ordering it should be 1st of 3 return values, no? Also, does go syntax permit not to assign a variable to returned value? Because if it doesn't, ordering doesn't make sense.

assignment mismatch: 1 variable but 2 values

This one is good. I would only specify that these are "2 return values".

@gopherbot

This comment has been minimized.

Copy link

commented Sep 4, 2018

Change https://golang.org/cl/133335 mentions this issue: cmd/compile: use "N variables but M values" error for OAS

@gopherbot gopherbot closed this in f7a633a Sep 4, 2018

@mdempsky

This comment has been minimized.

Copy link
Member

commented Sep 4, 2018

would only specify that these are "2 return values".

The current wording for x, y = three() is "2 variables but 3 values," so the consistent wording for x = three() is "1 variable but 3 values."

As I mentioned on the CL, I'm open to discussion about changing both instances to "3 return values" (or more precisely "3 result values," per the Go spec), but let's open a new issue for that. Personally, I think the current wording (as of f7a633a) is unambiguous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.