Skip to content

Allow shorter signed hex literals with appropriate type suffixes#11844

Merged
adityapatwardhan merged 4 commits intoPowerShell:masterfrom
vexx32:Hex/FixSignedLiterals
Apr 20, 2020
Merged

Allow shorter signed hex literals with appropriate type suffixes#11844
adityapatwardhan merged 4 commits intoPowerShell:masterfrom
vexx32:Hex/FixSignedLiterals

Conversation

@vexx32
Copy link
Copy Markdown
Collaborator

@vexx32 vexx32 commented Feb 13, 2020

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, 0xFFFFs now 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

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.


// If we're expecting a sign bit, remove the leading 0 added in ScanNumberHelper
if (!suffix.HasFlag(NumberSuffixFlags.Unsigned))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a dup of 3700 line?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, weird. Guessing a copy-paste error. Thanks for noticing that! I'll edit the duplicate segment out. 💖

@vexx32 vexx32 force-pushed the Hex/FixSignedLiterals branch from 2900fd4 to dfe7feb Compare February 13, 2020 06:24
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one minor comment.

Comment on lines +5015 to +5016
if (strNum == null)
{ return ScanGenericToken(c); }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 14, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Feb 14, 2020
@adityapatwardhan
Copy link
Copy Markdown
Member

@daxian-dbw The change is in the tokenizer, I would like you to have a look.

@adityapatwardhan adityapatwardhan merged commit ba53621 into PowerShell:master Apr 20, 2020
@vexx32 vexx32 deleted the Hex/FixSignedLiterals branch April 21, 2020 00:07
@ghost
Copy link
Copy Markdown

ghost commented Apr 23, 2020

🎉v7.1.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Dec 2, 2020

BackPort-7.0.x-Consider is asked in #14302

@adityapatwardhan
Copy link
Copy Markdown
Member

@SteveL-MSFT / @joeyaiello We need you comment about whether this should be back ported to 7.0.x?

@TravisEz13
Copy link
Copy Markdown
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

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Negative hex literal int16 throws parse error

6 participants