Add C#-style type accelerators and suffixes for ushort, uint, ulong, and short literals by vexx32 · Pull Request #7813 · PowerShell/PowerShell

vexx32 · 2018-09-18T17:10:42Z

PR Summary

This has been split off of #7509 at request of @iSazonov as that PR has become overly complex, and this portion of the PR was already approved by Powershell Committee. Re-submitting instead of working with #7575 due to messy rebase shenanigans that just... made a mess.

This fixes #3313.

Brief summary:

Adds the following numeric literal suffixes:

'u' suffix (uint/ulong)
's' suffix (short)
'ul' suffix (ulong)
'us' suffix (ushort)

Adds the following type accelerators:

[short]
[ushort]
[uint]
[ulong]

And of course, adds tests for all the above.

Comprehensive Summary:

Implements type suffixes s (short) and u (unsigned) to complement the existing l suffix.
- Suffixes can be combined as us or ul to result in ushort or ulong types.
When u suffix is used alone, uint will be returned unless value exceeds uint.MaxValue, in which case it will automatically resolve as ulong instead.
Adds new type accelerators to fit new numeric types better into the PS ecosystem:
- [short] => Int16
- [ushort] => UInt16
- [uint] => UInt32
- [ulong] => UInt64
Maintains parity with existing l / L suffix, where it will also perform rounding functions when applied to a real literal while also ensuring it still returns an appropriate uint or ulong value: 105.5u => 106
Defines a new [Flags] enum NumberSuffixFlags that takes care of one-or-more numerical type suffixes.
Use of u, l, s and valid combinations thereof will error out and register as an invalid numeric literal if their value after multiplication exceeds the type(s) contraints.
- This is intentional; if the user specifies a value that cannot be satisfied, such as 500sgb (500gb -as Int16)then the parser should not be expected to figure out what they meant.
Changes strNum parameter of TryGetNumberValue to ReadOnlySpan<char> to reduce the number of string allocations required for parsing.

PR Checklist

s -> short u -> unsigned

Update numeric parsing tests

vexx32 · 2018-09-18T19:20:10Z

@iSazonov I think this is tidier now. shudders I need to take a rebasing class! 😝

The remaining build errors appear to be build environment errors unrelated to this PR. ^^

The CodeFactor issues are (I think) not particularly serious, and I have a follow-up PR that will address the vast majority of those coming shortly.

iSazonov · 2018-09-19T11:59:12Z

@vexx32 To rebase you can use the pattern:

# rebase pull request on top of master
git chechout <branch>
git fetch PowerShell master
git pull --rebase PowerShell master
git push -f

You can catch a conflict but it is another story.

vexx32 · 2018-09-19T23:39:39Z

Reopened to restart CIs. Looks all good, I guess the Linux issues were resolved.

@iSazonov I have never had the good fortune to get a rebase to work without painstaking editing at every step along the way. Not sure why... I think I must be doing it wrong somehow. I need to read up on it! 😄

SteveL-MSFT

LGTM

iSazonov · 2018-09-26T05:55:19Z

@daxian-dbw Could you please look th PR? @vexx32 want to continue working on the code in follow PR.

daxian-dbw

About the comments regarding the tests, I think it makes sense to keep some of the existing tests. They can be put only in the #Standard numeric notation section, not in other sections for the new notations.

src/System.Management.Automation/engine/parser/tokenizer.cs

test/powershell/Language/Parser/Parser.Tests.ps1

daxian-dbw · 2018-09-27T23:53:26Z

Marked this PR with Breaking-Change label. The new type suffix makes some command names be parsed as numbers now, for example:

function 100u { "Yay" }
100u

daxian-dbw · 2018-09-28T00:07:41Z

@vexx32 I'm trying to tidy the PR description because the documentation work that follows up will need the accurate information.

'u' suffix (uint32)

By looking into the code, the suffix u doesn't guarantee uint32. It tries to fit the literal value into uin32 first, but if it fails, ulong will be used. Is taht implemetnation intentional? If so, please update the description.

I copied the Comprehensive Summary from #7509 as it's more relevant to this PR. But the summary is a little outdated compared with the implementation. Can you please update the summary? Thanks!

vexx32 · 2018-09-28T00:50:52Z

@daxian-dbw Yes, the automatic step-up to ulong is intentional. I felt that because u can be read simply as "unsigned" it makes the most sense to apply it in keeping with the existing patterns as best it can (i.e., a large integer currently will parse as long; thus, a large unsigned integer converts up to ulong when it exceeds uint32 bounds). I'll update the description and try to make sure everything fits with how it's intended to be. 😄

vexx32 · 2018-09-28T01:49:58Z

@daxian-dbw I think I got them all, but let me know if I missed anything! 😄

PR description has been cleaned up and made hopefully useful as a jumping off point for some proper documentation.

For Thoroughness!

vexx32 · 2018-09-28T13:47:34Z

Aww, I was enjoying the all-green. Appveyor's failing on a Test-Connection due to timeout.

daxian-dbw

LGTM. Thanks @vexx32 for your contribution! I will restart the AppVeyor CI.

vexx32 · 2018-09-28T18:04:13Z

Yay! 😄

Now I can get the next one ready for Hacktoberfest 😉

AppVeyor's still having difficulty with Test-Connection, it seems...

vexx32 added 4 commits September 18, 2018 13:04

Add CharTraits to suffix characters for additional numeric types

f9d9626

s -> short u -> unsigned

Add tokenizing logic for parsing short and unsigned values

2bccd8d

Add tests for parsing new numeric types

401a354

Update numeric parsing tests

Add type accelerators for friendly names of types

af1b63e

vexx32 requested review from BrucePay and daxian-dbw as code owners September 18, 2018 17:10

vexx32 mentioned this pull request Sep 18, 2018

Add C#-style type accelerators and suffixes for ushort, uint, ulong, and short literals. #7575

Closed

10 tasks

Add tests for new type accelerators

5271276

iSazonov approved these changes Sep 19, 2018

View reviewed changes

iSazonov requested a review from SteveL-MSFT September 19, 2018 11:36

vexx32 closed this Sep 19, 2018

vexx32 reopened this Sep 19, 2018

vexx32 mentioned this pull request Sep 19, 2018

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

Closed

daxian-dbw self-assigned this Sep 20, 2018

Codefactor space-after-braces fixes

0cbbed1

SteveL-MSFT approved these changes Sep 26, 2018

View reviewed changes

daxian-dbw requested changes Sep 27, 2018

View reviewed changes

daxian-dbw added the Breaking-Change breaking change that may affect users label Sep 27, 2018

daxian-dbw added the Documentation Needed in this repo Documentation is needed in this repo label Sep 27, 2018

vexx32 added 2 commits September 27, 2018 21:22

Address tokenizer comments

84b90be

Address comments on the parser tests

150eda3

Add should-fail case for super-high fallthrough on ushort

af72269

vexx32 added 2 commits September 28, 2018 08:48

Use pattern matching in out params

36ff4f6

Change var name to not conflict with prior pattern matching

5986b38

iSazonov approved these changes Sep 28, 2018

View reviewed changes

Add additional should-fail for us suffix with reals and multiplier

d1d46b2

For Thoroughness!

daxian-dbw approved these changes Sep 28, 2018

View reviewed changes

daxian-dbw merged commit 8f4b66a into PowerShell:master Sep 28, 2018

vexx32 deleted the Tokenizer/NumericSuffixes branch September 28, 2018 20:31

vexx32 mentioned this pull request Oct 1, 2018

Numeric Literal Suffixes documentation needs to be expanded MicrosoftDocs/PowerShell-Docs#2965

Closed

8 tasks

joeyaiello mentioned this pull request Oct 15, 2018

Docs needed for 'Add C#-style type accelerators and suffixes for ushort, uint, ulong, and short literals' MicrosoftDocs/PowerShell-Docs#3022

Closed

joeyaiello removed Documentation Needed in this repo Documentation is needed in this repo labels Oct 15, 2018

vexx32 mentioned this pull request Jul 1, 2019

function /(?<val>\d+)(?<timeUnit>)/([int]$val, [string]$timeUnit) { ... } #10042

Closed

vexx32 mentioned this pull request Jun 6, 2022

Binary AND returns incorrect value #4031

Closed

Comments

Conversation

vexx32 commented Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

vexx32 commented Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Sep 19, 2018

Uh oh!

vexx32 commented Sep 19, 2018

Uh oh!

SteveL-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Sep 26, 2018

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daxian-dbw commented Sep 27, 2018

Uh oh!

daxian-dbw commented Sep 28, 2018

Uh oh!

vexx32 commented Sep 28, 2018

Uh oh!

vexx32 commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vexx32 commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

vexx32 commented Sep 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vexx32 commented Sep 18, 2018 •

edited

Loading

vexx32 commented Sep 18, 2018 •

edited

Loading

vexx32 commented Sep 28, 2018 •

edited

Loading

vexx32 commented Sep 28, 2018 •

edited

Loading