Operation with native integer constant that might overflow in unchecked context should be treated as not constant#45581
Merged
cston merged 5 commits intodotnet:masterfrom Jul 3, 2020
Merged
Conversation
8f81795 to
a22dca6
Compare
AlekseyTs
reviewed
Jul 2, 2020
| <value>'{0}' cannot be sealed because containing 'record' is not sealed.</value> | ||
| </data> | ||
| <data name="WRN_ConstOutOfRangeChecked" xml:space="preserve"> | ||
| <value>Constant value '{0}' may overflow '{1}' (use 'unchecked' syntax to override)</value> |
Contributor
There was a problem hiding this comment.
may overflow [](start = 32, length = 12)
Do we want to add "at run-time"? #Closed
AlekseyTs
reviewed
Jul 2, 2020
| // Dev10 checks (minValue - 1) < value < (MaxValue + 1) + 1). | ||
| maySucceedAtRuntime = false; | ||
|
|
||
| // Dev10 checks (minValue - 1) < value < (maxValue + 1). |
Contributor
There was a problem hiding this comment.
maxValue [](start = 54, length = 8)
Does this change accurately reflect what Dev10 does?
AlekseyTs
reviewed
Jul 2, 2020
| } | ||
|
|
||
| private ConstantValue FoldConstantNumericConversion( | ||
| private ConstantValue? FoldConstantNumericConversion( |
Contributor
There was a problem hiding this comment.
FoldConstantNumericConversion [](start = 31, length = 29)
Are all existing consumers prepared able to deal with null as the result? #Resolved
Contributor
Author
chsienki
approved these changes
Jul 2, 2020
Member
|
Latest changes LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For conversions that depend on the platform, the conversion should be treated as not constant and computed at runtime rather than compile-time.
Fixes #42955
Fixes #45525
Fixes #45531
Test plan #38821