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

Literal overflow warnings should only apply to decimal. #24361

Closed
theemathas opened this issue Apr 13, 2015 · 5 comments
Closed

Literal overflow warnings should only apply to decimal. #24361

theemathas opened this issue Apr 13, 2015 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@theemathas
Copy link
Contributor

E.g. 0x8888i16 should not warn. Similar to binary operations | and &, hexadecimal, binary, and octal literals are often used to specify the concrete representation in memory whereas decimal literals are used to represent abstract values.

Also note that -(0x8000i16) which evaluates to -32768 (hex: 0x8000i16, which is the minimum value of i16) currently does not warn.

cc #22020

reincarnation of #23463

@steveklabnik steveklabnik added the A-diagnostics Area: Messages for errors, warnings, and lints label Apr 13, 2015
@steveklabnik
Copy link
Member

Triage: @rust-lang/lang , what do you think about this? This does still warn today, and I'm not sure where i stand, personally.

@eddyb
Copy link
Member

eddyb commented Jun 27, 2016

I would say that 0x8888u16 as i16 is the "correct" form, but I'm a bit torn.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 4, 2016

I agree with @eddyb

Also, regarding the -(0x8000i16) that does not warn: that falls at runtime on builds with overflow checking. E.g. debug builds.

@pnkfelix pnkfelix added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jul 4, 2016
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 22, 2017
@estebank
Copy link
Contributor

@rust-lang/lang should we close this given the resolution of #48073? There's no suggestion to write 0x8888u16 as i16, but #48432 could be expanded to do so. Otherwise, if we're not suggesting that, then we we should close this in my opinion.

@Mark-Simulacrum
Copy link
Member

I'm going to go ahead and close this as I think we've essentially intentionally chosen a path here and I believe the suggestion has been implemented, if indirectly, by showing what the literal in i16 would be. It seems like largely writing the "end result" or allowing the lint is an acceptable tradeoff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants