★ wanayoo — archive 1999 https://github.com/PowerShell/PowerShell/pull/7901Nouvelle recherche | Portail wanayoo
Skip to content

/PowerShell

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

Add support for Byte Literals #7901

Merged
merged 10 commits into from Oct 11, 2018
+126 −9

Conversation

Reviewers

@TravisEz13 TravisEz13

@iSazonov iSazonov

@daxian-dbw daxian-dbw

@BrucePay BrucePay

@SteveL-MSFT SteveL-MSFT

@SteveL-MSFT SteveL-MSFT

Assignees

@iSazonov iSazonov

Labels
Breaking-Change CL-General Committee-Reviewed
Projects
None yet
Milestone
No milestone
5 participants
@vexx32 @iSazonov @SteveL-MSFT @TravisEz13 @joeyaiello
@vexx32
Copy link
Contributor

commented Oct 1, 2018 •
edited

PR Summary

  • Adds y suffix that is used to specify a numeric literal as the sbyte data type.
    • Can be combined with the existing u suffix as uy to specify the byte data type
  • Adds tests to ensure the parsing for these works correctly.

Examples:

PS> 100y
100
PS> 100y.GetType().FullName
System.SByte
PS> 100uy
100
100uy.GetType().FullName
System.Byte
PS> 300uy
# Error: Invalid numeric literal

N.B.: I believe this will be a breaking change, as it means some command names (specifically, numerals suffixed with a y such as 100y) will now be interpreted as numerals instead of potential function or command names.

Ref: #7557 -- This will resolve the first part of that issue. The second portion of that issue will be solved in a follow-up PR where binary literal support is added.

PR Checklist

@vexx32 vexx32 requested review from BrucePay and daxian-dbw as code owners Oct 1, 2018

@iSazonov iSazonov added Breaking-Change Documentation Needed labels Oct 1, 2018

@iSazonov iSazonov changed the title Tokenizer Support for Byte Literals Add support for Byte Literals Oct 1, 2018

@iSazonov

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Oct 1, 2018

@vexx32 Please open Issue in docs repo and add link to the PR description
@iSazonov

iSazonov reviewed Oct 1, 2018
View changes

Show resolved Hide resolved src/System.Management.Automation/engine/parser/tokenizer.cs
Show resolved Hide resolved test/powershell/Language/Parser/Parser.Tests.ps1 Outdated

@vexx32 vexx32 referenced this pull request Oct 1, 2018

Closed

Numeric Literal Suffixes documentation needs to be expanded #2965

2 of 8 tasks complete
@iSazonov

iSazonov reviewed Oct 1, 2018
View changes

Show resolved Hide resolved src/System.Management.Automation/engine/parser/tokenizer.cs Outdated
@iSazonov

iSazonov approved these changes Oct 1, 2018
View changes

@vexx32

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Oct 2, 2018

CodeFactor issues remaining are just in missing documentation in the enum; I can either add that to this PR or address it in follow-up PR with the rest of the changes. I would prefer to keep this one minimal, however. 😄
@iSazonov

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Oct 3, 2018

@vexx32 Feel free to address CodeFactor issues.

@vexx32 vexx32 force-pushed the vexx32:Tokenizer/ByteLiterals branch Oct 3, 2018

@vexx32

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Oct 3, 2018 •
edited

Rebased for CIs && added <summary> headers for CodeFactor 😄

vexx32 added some commits Sep 30, 2018

@vexx32
Add NumberSuffixFlag enty for SignedByte
Loading status checks…
824a96a
@vexx32
Add explicit names for flag combinations
Loading status checks…
a98c5da
@vexx32
Add CharTraits for typesuffix on `y`
Loading status checks…
d1f5cb3
@vexx32
Add ScanNumberHelper cases for byte suffixes 
Use specific combined flag names in parsing cases
Add (s)byte parsing cases for TryGetNumberValue
Loading status checks…
cd185ec
@vexx32
Add unsigned byte tests
Loading status checks…
9838504
@vexx32
Add signed byte tests
Loading status checks…
820b688
@vexx32
Fix spacing issue
Loading status checks…
d1ac03b
@vexx32
Remove inapplicable binary parsing tests
Loading status checks…
7d557ea
@vexx32
Shortcut parsing fail for (s)byte when multiplier is set 
Remove impossible tests
Rework byte parsing shortcut

Fix failing test scenario caused by hex parse handling of multiplier

Simplify return by ignoring multuplier for (s)byte-suffixed values

Add comment for negative multiplier possibility

Still need to utilise multiplier for signed byte case
Multiplier can determine negative in signed byte cases with hex

Adjust byte parse shortcuts
Add comments
Loading status checks…
fd93c3b
@vexx32
Address CodeFactor issues 
Add summary headers to enum and values
Loading status checks…
d664649

@vexx32 vexx32 force-pushed the vexx32:Tokenizer/ByteLiterals branch to d664649 Oct 3, 2018

@iSazonov

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Oct 4, 2018

@daxian-dbw Could you please review the next step PR? After that we could continue and implement previously planned performance optimizations for the code in follow PR.

@iSazonov iSazonov requested review from SteveL-MSFT and TravisEz13 Oct 8, 2018

@iSazonov iSazonov self-assigned this Oct 8, 2018

@TravisEz13 TravisEz13 added the Review - Committee label Oct 8, 2018

@TravisEz13

TravisEz13 approved these changes Oct 8, 2018
View changes

@vexx32

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Oct 8, 2018

@TravisEz13 you fellas are free to pore over it one more time, but just in case you weren't aware / didn't recall, it has been discussed a good bit and reviewed by the committee in terms of the concept over in #7557 😄
@SteveL-MSFT

This comment has been minimized.

Sign in to view
Copy link
Member

commented Oct 8, 2018

@TravisEz13 is there something specific you need the @PowerShell/powershell-committee to review for this PR?
@TravisEz13

This comment has been minimized.

Sign in to view
Copy link
Member

commented Oct 8, 2018

@SteveL-MSFT This is marked as a breaking change. The issue linked is not marked as reviewed. Looking back at the issue, it looks like this portion of the design was reviewed. Is that correct?
@vexx32

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Oct 8, 2018

@TravisEz13 sorry for the confusion; I requested the committee further review the ensuing discussion regarding possibly supporting underscore literal syntax and native BigInteger values. This portion was reviewed, aye. 😃

@TravisEz13 TravisEz13 added Committee-Reviewed and removed Review - Committee labels Oct 8, 2018

@iSazonov

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Oct 9, 2018

@daxian-dbw Could you please look the PR before we merge?

@iSazonov iSazonov merged commit e660e96 into PowerShell:master Oct 11, 2018
8 checks passed

8 checks passed

CodeFactor 7 issues fixed. 2 issues found.
Details
PowerShell-CI-linux #PR-7901-20181003.03 succeeded
Details
PowerShell-CI-macos #PR-7901-20181003.03 succeeded
Details
PowerShell-CI-spelling #PR-7901-20181003.03 succeeded
Details
PowerShell-CI-windows #PR-7901-20181003.03 succeeded
Details
WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.
Details

@joeyaiello joeyaiello referenced this pull request Oct 15, 2018

Closed

Docs needed for 'Add support for Byte Literals' #3018

@joeyaiello joeyaiello removed Documentation Needed labels Oct 15, 2018

@vexx32 vexx32 referenced this pull request Nov 15, 2018

Closed

Allow binary numbers, byte literals, and BigInteger? #7557

@vexx32 vexx32 deleted the vexx32:Tokenizer/ByteLiterals branch Jan 22, 2019

@iSazonov iSazonov added the CL-General label Feb 1, 2019

adityapatwardhan pushed a commit to adityapatwardhan/PowerShell that referenced this pull request Apr 9, 2019

@vexx32 @iSazonov
Add support for Byte Literals (PowerShell#7901) 
- Adds y suffix that is used to specify a numeric literal as the sbyte data type. 
- Can be combined with the existing u suffix as uy to specify the byte data type.
31e90ac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.