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

go/types: string(1 << s) should be an error #26096

Open
griesemer opened this issue Jun 27, 2018 · 4 comments
Open

go/types: string(1 << s) should be an error #26096

griesemer opened this issue Jun 27, 2018 · 4 comments
Assignees
Labels
Milestone

Comments

@griesemer
Copy link
Contributor

@griesemer griesemer commented Jun 27, 2018

package main

func main() {
	var s uint
	_ = string(1 << s)
}

(https://play.golang.org/p/owHsmdZd32v) is an error: The 1 in 1 << s assumes the type it would have without the shift, which is string. Both cm/compile and gccgo correct report an error.

go/types appears to accept it.

@griesemer griesemer added this to the Go1.12 milestone Jun 27, 2018
@griesemer griesemer self-assigned this Jun 27, 2018
@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Jun 27, 2018

In this case (string(number)) where the string conversion is not simply changing the type of the argument but actively changing its value and representation, it would make sense for the shift rules to not look at the context. Or turning it around, maybe this kind of conversion should not be permitted in the first place, as proposed via #3939.

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Jun 27, 2018

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Jun 28, 2018

Also related: #21982

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Sep 12, 2018

On (go/types) conversions.go:61 we select untyped int (the current type of 1 in string(1 << s)) as type for 1 (i.e., we don't change it). If we don't do anything special in this case (integer to string conversions), the type of 1 will become srtring and we get the expected error message.

Unfortunately, doing nothing there causes regular conversions (such as string('a')) to break because the argument to the conversion is not compatible with string anymore.

Fixing this requires a bit more subtle changes. Postponing for now since this is a) unlikely to occur in real programs, b) there are easy work-arounds, and c) this doesn't prevent go/types from accepting legal programs.

The issue may also become moot if we remove string(int/rune) conversions (#3939).

@griesemer griesemer removed this from the Go1.12 milestone Sep 12, 2018
@griesemer griesemer added this to the Unplanned milestone Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant