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: consistently report overflow for untyped constant arguments of to print/println - needs release notes #49216

Closed
griesemer opened this issue Oct 29, 2021 · 2 comments
Milestone

Comments

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 29, 2021

Problem

The pre-1.18 compiler treats untyped constants of rune kind (such as 'a', '1' << 10, etc.) passed as arguments to the built-in functions print and println differently than other constants if those values overflow the 'rune' range.

Example, compiled using pre-1.18 compiler:

var _ = '1' << 32  // constant 210453397504 overflows rune
println('1' << 32) // no error

Example, compiled using 1.18 compiler:

var _ = '1' << 32  // cannot use '1' << 32 (untyped rune constant 210453397504) as rune value in variable declaration (overflows)
println('1' << 32) // cannot use '1' << 32 (untyped rune constant 210453397504) as rune value in argument to println (overflows)

The pre-1.18 compiler accepts an int64 value if an int32 value is not sufficient, rather than reporting an error as would be warranted by the usual assignment and parameter passing rules.

Proposal

The compiler should consistently apply parameter passing rules from now on, even for arguments to the "magic" built-in functions print and println.

For existing code that doesn't compile anymore there are a couple of work-arounds:

  1. Use a conversion (e.g., int64('1' << 32)).
  2. Compile using the 1.17 compiler by setting the -G=0 compiler flag. This doesn't require code changes.

Per the spec:

Current implementations provide several built-in functions useful during bootstrapping. These functions are documented for completeness but are not guaranteed to stay in the language.

Thus, arguably a (non-temporary) correct program should not be relying on these functions in the first place. At the very least we should be in the position to make this minor adjustments.

Implementation

If the proposal is accepted, nothing needs to be done; this is already the new behavior of the compiler.
If the proposal is not accepted, the compiler needs to be adjusted such that untyped rune constant arguments to print and println are treated as int64 values if they overflow as rune (i.e., int32) values.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 3, 2021

This is spec-compatible, and it is also compatible with go vet.
(go vet would not have succeeded on code that is newly rejected here.)

Since there is no spec change, we don't really need a proposal here,
but we should be ready to implement the old behavior
if there are significant problems during the beta/rc period.

Leaving open to document in release notes.

@rsc rsc changed the title proposal: cmd/compile: consistently report overflow for untyped constant arguments of to print/println cmd/compile: consistently report overflow for untyped constant arguments of to print/println Nov 3, 2021
@rsc rsc removed this from Incoming in Proposals Nov 3, 2021
@rsc rsc added NeedsFix and removed Proposal labels Nov 3, 2021
@griesemer griesemer added Documentation and removed NeedsFix release-blocker labels Nov 3, 2021
@griesemer griesemer changed the title cmd/compile: consistently report overflow for untyped constant arguments of to print/println cmd/compile: consistently report overflow for untyped constant arguments of to print/println - needs release notes Nov 3, 2021
@griesemer griesemer self-assigned this Nov 3, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 22, 2021

Change https://golang.org/cl/366275 mentions this issue: doc: document new overflow error for some untyped arguments to print/ln

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants