Allow shorter signed hex literals with appropriate type suffixes #11844
Conversation
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
See PowerShell#11823 for context and discussion. This change allows shorter hex literals to be treated as signed, if the specified type suffix is a signed type with correct hex lengths. For example, 0xFFFFs now parses correctly to -1 instead of erroring.
ghost
assigned
PoshChan
reviewed
Feb 13, 2020
PoshChan
reviewed
Feb 13, 2020
PoshChan
reviewed
Feb 13, 2020
iSazonov
reviewed
Feb 13, 2020
|
|
||
|
|
||
| // If we're expecting a sign bit, remove the leading 0 added in ScanNumberHelper | ||
| if (!suffix.HasFlag(NumberSuffixFlags.Unsigned)) |
Collaborator
There was a problem hiding this comment.
Why is this a dup of 3700 line?
Collaborator
Author
There was a problem hiding this comment.
Oh, weird. Guessing a copy-paste error. Thanks for noticing that! I'll edit the duplicate segment out. 💖
PoshChan
reviewed
Feb 13, 2020
PoshChan
reviewed
Feb 13, 2020
PoshChan
reviewed
Feb 13, 2020
vexx32
force-pushed
the
Hex/FixSignedLiterals
branch
from
2900fd4 to
dfe7feb
Compare
iSazonov
approved these changes
Feb 14, 2020
Collaborator
iSazonov
left a comment
iSazonov
left a comment
There was a problem hiding this comment.
LGTM with one minor comment.
Comment on lines
5015
to
5016
| if (strNum == null) | ||
| { return ScanGenericToken(c); } |
iSazonov
added
the
CL-General
Member
|
@daxian-dbw The change is in the tokenizer, I would like you to have a look. |
adityapatwardhan
approved these changes
Apr 7, 2020
daxian-dbw
approved these changes
Apr 20, 2020
|
🎉 Handy links: |
Closed
12 tasks
Collaborator
|
BackPort-7.0.x-Consider is asked in #14302 |
adityapatwardhan
added
BackPort-7.0.x-Consider
and removed
BackPort-7.0.x-Consider
labels
Member
|
@SteveL-MSFT / @joeyaiello We need you comment about whether this should be back ported to 7.0.x? |
Member
|
@SteveL-MSFT Do we want to backport this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
PR Summary
This change allows shorter hex literals to be treated as signed, if the specified type suffix is a signed type with correct hex lengths. For example,
0xFFFFsnow parses correctly to -1 instead of erroring. Default behaviour without a specific suffix and/or with explicit unsigned suffixes remains as it was.PR Context
Resolves #11823. See the issue for further context and discussion.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.