GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add C#-style type accelerators and suffixes for ushort, uint, ulong, and short literals by vexx32 · Pull Request #7813 · PowerShell/PowerShell

Conversation

@vexx32

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) commented Sep 18, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

PR Summary

This has been split off of #7509 at request of Ilya (@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

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • User-facing Documentation needed
      • Issue filed - Issue link:
  • Testing - New and feature
    • Not Applicable or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
      • Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mklement0 and Nacimota reacted with hooray emoji
All reactions
  • 🎉 2 reactions
Rain Sallow (/u/ta11ow) (vexx32) added 4 commits September 18, 2018 13:04
@vexx32
Add CharTraits to suffix characters for additional numeric types
f9d9626
s -> short
u -> unsigned
@vexx32
Add tokenizing logic for parsing short and unsigned values
2bccd8d
@vexx32
Add tests for parsing new numeric types
401a354
Update numeric parsing tests
@vexx32
Add type accelerators for friendly names of types
af1b63e
@vexx32
Rain Sallow (/u/ta11ow) (vexx32) requested review from Bruce Payette (BrucePay) and Dongbo Wang (daxian-dbw) as code owners September 18, 2018 17:10
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Sep 18, 2018
Add C#-style type accelerators and suffixes for ushort, uint, ulong, and short literals. #7575
Closed
6 of 10 tasks
@vexx32
Add tests for new type accelerators
5271276
@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Sep 18, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

Ilya (@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov
Ilya (iSazonov) approved these changes Sep 19, 2018
View reviewed changes
@iSazonov
Ilya (iSazonov) requested a review from Steve Lee (SteveL-MSFT) September 19, 2018 11:36
@iSazonov

Ilya (iSazonov) commented Sep 19, 2018

Copy link
Copy Markdown
Collaborator

Rain Sallow (/u/ta11ow) (@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) closed this Sep 19, 2018
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) reopened this Sep 19, 2018
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Sep 19, 2018
Allow binary numbers, byte literals, and BigInteger? #7557
Closed
@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Sep 19, 2018

Copy link
Copy Markdown
Collaborator Author

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

Ilya (@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! 😄

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw Dongbo Wang (daxian-dbw) self-assigned this Sep 20, 2018
@vexx32
Codefactor space-after-braces fixes
0cbbed1
SteveL-MSFT
Steve Lee (SteveL-MSFT) approved these changes Sep 26, 2018
View reviewed changes

@SteveL-MSFT Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@iSazonov

Ilya (iSazonov) commented Sep 26, 2018

Copy link
Copy Markdown
Collaborator

Dongbo Wang (@daxian-dbw) Could you please look th PR? Rain Sallow (/u/ta11ow) (@vexx32) want to continue working on the code in follow PR.

daxian-dbw reacted with thumbs up emoji vexx32 reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Sep 27, 2018
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/System.Management.Automation/engine/parser/tokenizer.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/parser/tokenizer.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/System.Management.Automation/engine/parser/tokenizer.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/powershell/Language/Parser/Parser.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/powershell/Language/Parser/Parser.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/powershell/Language/Parser/Parser.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/powershell/Language/Parser/Parser.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/powershell/Language/Parser/Parser.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/powershell/Language/Parser/Parser.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw Dongbo Wang (daxian-dbw) added the Breaking-Change breaking change that may affect users label Sep 27, 2018
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Sep 27, 2018

Copy link
Copy Markdown
Member

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
vexx32 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw Dongbo Wang (daxian-dbw) added the Documentation Needed in this repo Documentation is needed in this repo label Sep 27, 2018
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Sep 28, 2018

Copy link
Copy Markdown
Member

Rain Sallow (/u/ta11ow) (@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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Sep 28, 2018

Copy link
Copy Markdown
Collaborator Author

Dongbo Wang (@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. 😄

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Rain Sallow (/u/ta11ow) (vexx32) added 2 commits September 27, 2018 21:22
@vexx32
Address tokenizer comments
84b90be
@vexx32
Address comments on the parser tests
150eda3
@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Sep 28, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

Dongbo Wang (@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.

daxian-dbw reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vexx32
Add should-fail case for super-high fallthrough on ushort
af72269
Rain Sallow (/u/ta11ow) (vexx32) added 2 commits September 28, 2018 08:48
@vexx32
Use pattern matching in out params
36ff4f6
@vexx32
Change var name to not conflict with prior pattern matching
5986b38
iSazonov
Ilya (iSazonov) approved these changes Sep 28, 2018
View reviewed changes
@vexx32
Add additional should-fail for us suffix with reals and multiplier
d1d46b2
For Thoroughness!
@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Sep 28, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Sep 28, 2018
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thanks Rain Sallow (/u/ta11ow) (@vexx32) for your contribution! I will restart the AppVeyor CI.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vexx32

Rain Sallow (/u/ta11ow) (vexx32) commented Sep 28, 2018

Copy link
Copy Markdown
Collaborator Author

Yay! 😄

Now I can get the next one ready for Hacktoberfest 😉

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

daxian-dbw reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 8f4b66a into PowerShell:master Sep 28, 2018
@vexx32
Rain Sallow (/u/ta11ow) (vexx32) deleted the Tokenizer/NumericSuffixes branch September 28, 2018 20:31
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Oct 1, 2018
Numeric Literal Suffixes documentation needs to be expanded MicrosoftDocs/PowerShell-Docs#2965
Closed
2 of 8 tasks
@joeyaiello Joey Aiello (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 Joey Aiello (joeyaiello) removed Documentation Needed in this repo Documentation is needed in this repo labels Oct 15, 2018
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Jul 1, 2019
function /(?<val>\d+)(?<timeUnit>)/([int]$val, [string]$timeUnit) { ... } #10042
Closed
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Jun 6, 2022
Binary AND returns incorrect value #4031
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@vexx32 @daxian-dbw
Add C#-style type accelerators and suffixes for ushort, uint, ulong, …
b4c32a3
…and short literals (PowerShell#7813)

[breaking change]
Add the following type accelerators: [short], [ushort], [uint], [ulong].
Add the following numeric literal suffixes:
 - 'u' suffix (uint/ulong)
 - 's' suffix (short)
 - 'ul' suffix (ulong)
 - 'us' suffix (ushort)
@Yihang-Chen-math Yihang-Chen-math mentioned this pull request Nov 7, 2026
Windows: generated PowerShell scripts pass AST validation but fail in explicitly selected PowerShell 5.1 openai/codex#60081
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@BrucePay Bruce Payette (BrucePay) Awaiting requested review from BrucePay

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@daxian-dbw Dongbo Wang (daxian-dbw)

Labels

Breaking-Change breaking change that may affect users

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PowerShell doesn't understand unsigned numbers

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@vexx32 @iSazonov @daxian-dbw @SteveL-MSFT @joeyaiello
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.