Skip to content

Consolidate polyfills and conditional compilation; fix seekability defects - #320

Merged
sebastienros merged 8 commits into
sebastienros:mainfrom
lahma:perf/polyfill-consolidation
Aug 3, 2026
Merged

Consolidate polyfills and conditional compilation; fix seekability defects#320
sebastienros merged 8 commits into
sebastienros:mainfrom
lahma:perf/polyfill-consolidation

Conversation

@lahma

@lahma lahma commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

What

Consolidates Parlot's conditional-compilation surface, gives the downlevel target frameworks test
coverage for the first time, and fixes three defects that only became visible once the guards were
readable.

Parlot targets net472;netstandard2.0;net8.0;net10.0. Over that exact set, NET, NETCOREAPP,
NET6_0_OR_GREATER, NET7_0_OR_GREATER and NET8_0_OR_GREATER all denote the same two frameworks.
All five spellings were in use.

before after
#if / #elif blocks in src/ 60 41
distinct TFM guard spellings 8 3 (two distinct sets)
target frameworks executed by tests net8.0, net10.0 net8.0, net10.0, net472
tests run 1723 2465

Commits

Each commit records whether it moves the net10.0 leg. Reviewable in order.

  1. Normalize the guard vocabulary. Every TFM guard becomes NET8_0_OR_GREATER. Set-identical
    over the actual TFM list — verified with ilspycmd -il, whose output is byte-identical before and
    after on both the newest (net10.0) and the oldest (net472) leg.
  2. net472 test leg + Windows CI job. Parlot.Tests targeted only net8.0/net10.0 and CI ran
    ubuntu-only, so no test in the repository had ever executed a downlevel code path. The leg is
    OS-conditioned in the csproj, so dotnet build / dotnet test are unchanged on Linux and Windows
    picks it up for free. Three portability fixes outside src/Parlot that this surfaced are in the
    same commit.
  3. Delete Polyfill.cs. Both members had zero call sites. Append<T> shadowed
    Enumerable.Append — which ships in netstandard2.0 and .NET Framework 4.7.1+ — with a version
    that copies the whole source into a List<T> first.
  4. Backfill missing BCL members as C# 14 extension members. Numbers.cs had thirteen #if/#else
    pairs whose two branches said the same thing twice. It is now written as if the project only
    targeted net10.0, with the downlevel bodies in src/Parlot/Polyfills/. 14 blocks become 1.
  5. Delete unreachable Half handling. An #if NET8_0_OR_GREATER branch nested inside the
    #else of #if NET8_0_OR_GREATER, plus the class it was the only consumer of.
  6. Fix Terms.AnyOf(ReadOnlySpan<char>) losing CanSeek and wrapping SkipWhiteSpace twice.
  7. Fix ListOfChars never being seekable downlevel.
  8. Use char.IsAsciiDigit/IsAsciiHexDigit for the digit predicates.

Defects fixed

Terms.AnyOf(ReadOnlySpan<char>) was not seekable on net8.0+. It delegated to
Terms.AnyOf(SearchValues<char>), and that constructor cannot recover the source chars, so it left
CanSeek = false and ExpectedChars empty. Literals.AnyOf, Terms.NoneOf and the net472 #else
leg were all seekable. It also produced SkipWhiteSpace(SkipWhiteSpace(literal)).

ListOfChars was never seekable at all on net472/netstandard2.0: the constructor tested the
fields _minSize and _negate before assigning them from the parameters, so the condition was
always 0 > 0 && !false. Found by the new seekability test on the new net472 leg — the first defect
that leg caught, and one the test suite could not previously reach.

Benchmarks

char.IsAscii* versus SearchValues.Contains for the digit sets (MediumRun, 15 iterations ×
2 launches). DecimalDigits is "0123456789" and HexDigits is "0123456789abcdefABCDEF", so the
BCL predicates are exact equivalents, asserted over all 65536 char values by two new tests:

Method Mean Error StdDev
SearchValuesContains_IsDecimalDigit_True 0.1911 ns 0.0066 ns 0.0092 ns
SearchValuesContains_IsDecimalDigit_False 0.1945 ns 0.0094 ns 0.0132 ns
IsAsciiDigit_True 0.0159 ns 0.0165 ns 0.0246 ns
IsAsciiDigit_False 0.0138 ns 0.0231 ns 0.0323 ns
SearchValuesContains_IsHexDigit_True 0.1163 ns 0.0215 ns 0.0309 ns
SearchValuesContains_IsHexDigit_False 0.2146 ns 0.0670 ns 0.0983 ns
IsAsciiHexDigit_True 0.0000 ns 0.0000 ns 0.0000 ns
IsAsciiHexDigit_False 0.0074 ns 0.0061 ns 0.0089 ns

The IsAscii* rows sit at the resolution floor — they inline to a couple of instructions — so read
them as "free" rather than as exact values. IsIdentifierStart/IsIdentifierPart are deliberately
unchanged: they have no BCL equivalent, and the pre-existing benchmark in the same file shows
SearchValues.Contains is the best option for those sets.

For Terms.AnyOf, a benchmark comparing the single- and double-wrapped shapes is included. The
direction is consistent (7 of 8 measurements favour the single wrap, ratios 1.02–1.40) but the
magnitude is within noise on my machine, so the argument for that commit is the lost seekability and
the divergence from every sibling factory, not the layer cost.

Declined polyfills

Recorded with a comment at each site so a later sweep does not undo them.

API Why not
SearchValues<char> Missing type, and a polyfill degrades IndexOfAny/IndexOfAnyExcept to a per-char loop — a regression in Scanner's hottest loops. The #else legs there are different algorithms, not naive scans.
INumber<T> Missing type; gates a whole modern-only lane and a public generic constraint.
Half Missing type downlevel.
string.GetHashCode(span, StringComparison) for TextSpan.GetHashCode The API is .NET Core 2.1+, and so is the string.GetHashCode(StringComparison) a naive polyfill body would call. A correct downlevel body allocates the string anyway — which is what the #else already does, without the indirection.
new string(ReadOnlySpan<char>) Constructors cannot be extension members; the one call site now uses ToString().

Verification

  • dotnet build -c Release — all four legs, 0 warnings (TreatWarningsAsErrors is on).
  • dotnet test -c Release — 2465 passed, 0 failed, across net472 / net8.0 / net10.0.
  • ilspycmd -il diff on src/Parlot for net10.0 and net472 between commits. Commits 1 and 2 leave
    both legs byte-identical. Commit 4's only modern-leg change is the aggressiveinlining flag now on
    ThrowHelper.ThrowIfNull, which had been a plain non-inlined wrapper.
  • Benchmarks target net10.0 only, so a commit leaving that leg's preprocessed source identical is not
    measurable by this repository; the IL diff is the evidence for those commits.

Notes for follow-up (not in this PR)

The whole NumberLiteralBase<T> family in Fluent/NumberLiteralBase.cs is only reachable from the
#else leg of NumberLiterals.CreateNumberLiteralParser<T>, so net8.0/net10.0 compile it in and
never construct it. Guarding the family with #if !NET8_0_OR_GREATER would drop ~13 types from the
modern assemblies, but it is a larger diff than belongs here.


🤖 Generated with Claude Code

lahma and others added 8 commits August 2, 2026 21:02
Parlot targets net472;netstandard2.0;net8.0;net10.0. Over that exact set,
`NET`, `NETCOREAPP`, `NET6_0_OR_GREATER`, `NET7_0_OR_GREATER` and
`NET8_0_OR_GREATER` all denote the same two frameworks, {net8.0, net10.0}.
Five spellings for one set makes the guards impossible to audit and makes
dropping a TFM a judgement call instead of a mechanical edit.

Collapse all five to `NET8_0_OR_GREATER`, so every remaining TFM guard names
a framework the repository actually targets and the negative form
`!NET8_0_OR_GREATER` reads as "the legs that lack this". `NET10_0_OR_GREATER`,
`DEBUG` and `SOURCE_GENERATOR` denote different sets and are untouched, as are
the guards inside the source generator's emitted string literals.

Every substitution is set-identical, so no leg's preprocessed source moves.
Verified: `ilspycmd -il` output for both the newest (net10.0) and the oldest
(net472) legs is byte-identical before and after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parlot ships net472 and netstandard2.0, but Parlot.Tests targeted only
net8.0;net10.0 and CI ran ubuntu-only, so *no test in the repository ever
executed a downlevel code path*. Every `#else` branch and every future polyfill
would ship untested.

Add a net472 leg to Parlot.Tests and Samples, conditioned on the OS in the
csproj so `dotnet build` and `dotnet test` are unchanged on Linux, and make the
CI workflow a matrix over ubuntu-latest and windows-latest. Windows then runs
the leg for free; Linux never sees it.

727 tests now execute on net472, on top of the 1723 that already ran on
net8.0/net10.0.

Three portability fixes the new leg surfaced, all outside src/Parlot:

- Samples/Json/JsonParser.cs and Parlot.Tests/OperatorsTests.cs used
  `Dictionary<TKey, TValue>(IEnumerable<KeyValuePair<TKey, TValue>>)`, which is
  .NET Core 2.0+. On .NET Framework that call does not fail to compile in the
  obvious way -- it silently binds to the `(int capacity)` overload and fails
  with a confusing conversion error. Build the dictionary explicitly instead.
- Parlot.Tests/UtilityTypesTests.cs used `new string(ReadOnlySpan<char>)`
  (.NET Core 2.1+). Constructors cannot be backfilled as extension members, so
  the call site now uses `ToString()`.

The source generator's emitted code already carried its own
`#if NET8_0_OR_GREATER` / `#else` pairs and compiled on net472 unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
src/Parlot/Polyfill.cs had two members and zero call sites anywhere in src/ or
test/:

- `Append<T>(this IEnumerable<T>, T)` should never have existed. Enumerable.Append
  ships in netstandard2.0 and .NET Framework 4.7.1+ (verified against the net472
  reference assembly), so this shadowed a real BCL API with a version that copies
  the entire source into a List<T> first -- a silent O(n) allocation on exactly
  the frameworks it claimed to help.
- `Create<TState>(this string, ...)` invented a `SpanAction<T[], TArg>` delegate
  over char[] rather than Span<char>, so it did not mirror string.Create at all.

Deleting the file also removes the `#if` / `using System.Linq;` / `#endif` blocks
in Error.cs, If.cs, When.cs, WhenFollowedBy.cs, WhenNotFollowedBy.cs and
TextBefore.cs. None of those six files uses a single LINQ operator, and ~20 other
files in the same assembly import System.Linq unconditionally, so the guards
protected nothing.

Net effect: 8 conditional blocks and one type removed. IL diff confirms the only
change on net10.0 is the disappearance of the empty `Parlot.Polyfill` class; on
net472 the class and its delegate are gone and nothing else moved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Numbers.cs carried thirteen #if/#else pairs whose then- and else-branches said
the same thing twice: `T.TryParse(span, ...)` versus
`T.TryParse(span.ToString(), ...)`. That is a polyfill written thirteen times at
the call site instead of once in a polyfill.

Add src/Parlot/Polyfills/, guarded `#if !NET8_0_OR_GREATER`:

- NumberPolyfills backfills the twelve span-based TryParse overloads (.NET Core
  2.1 / netstandard2.1) on the numeric primitives and BigInteger.
- ArgumentNullExceptionPolyfills backfills ArgumentNullException.ThrowIfNull
  (.NET 6), with [NotNull] and [CallerArgumentExpression] supplied downlevel by
  PolySharp, which is already a GlobalPackageReference.

Numbers.cs and ThrowHelper.cs are now written as if the project only targeted
net10.0: 14 conditional blocks become 1 (the Half overload, which is a missing
*type* and cannot be polyfilled). Verified in the decompiled output that
net10.0 binds `int.TryParse` / `ArgumentNullException.ThrowIfNull` directly
while net472 binds NumberPolyfills / ArgumentNullExceptionPolyfills -- the
newest runtime keeps the BCL implementation, which is the point.

The receiver type is erased when a static extension member is lowered, which
normally forces one container per receiver to avoid CS0111. It does not bite
here because every TryParse overload differs in its `out` parameter type; there
is a comment on the container saying so, so the next sweep does not "fix" it.

Declined, with the reason recorded at the site:

- TextSpan.GetHashCode keeps its guard. Both candidate APIs
  (CompareInfo.GetHashCode(span, CompareOptions) and
  string.GetHashCode(span, StringComparison)) are .NET Core 2.1+, and a correct
  downlevel body has to allocate the string anyway -- which is exactly what the
  #else already does, without the indirection.
- Half, INumber<T> and SearchValues<char> are missing *types*, not members.

net10.0 IL is unchanged except for the aggressiveinlining flag now on
ThrowHelper.ThrowIfNull, which had been a plain non-inlined wrapper.
All 2450 tests pass, 727 of them on net472.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two dead pieces, both made visible by normalizing the guard vocabulary.

NumberLiterals.CreateNumberLiteralParser had an `#if NET8_0_OR_GREATER` branch
for Half nested *inside the #else of* `#if NET8_0_OR_GREATER`. That branch could
never compile on any target framework: on net8.0+ the outer then-branch takes
the INumber<T> lane, and on net472/netstandard2.0 System.Half does not exist.

HalfNumberLiteral was that branch's only consumer, so it was compiled into the
net8.0/net10.0 assemblies and never constructed by anything.

Removes one type from the modern legs and replaces the dead branch with a
comment explaining why there is no Half case there.

Note the `#if NET8_0_OR_GREATER` guards in src/Samples and test/Parlot.Tests
were also always-true before this branch; they are load-bearing again now that
both projects have a net472 leg, so they stay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wice

Terms.AnyOf(ReadOnlySpan<char>) delegated to Terms.AnyOf(SearchValues<char>),
which already wraps its literal in SkipWhiteSpace. The result was
SkipWhiteSpace(SkipWhiteSpace(literal)) -- unlike Terms.NoneOf(span) directly
below it, unlike every other Terms.* factory, and unlike its own #else leg on
net472.

The extra parser layer was the visible half of the problem. The costly half is
that routing through the SearchValues<char> constructor throws away the source
chars: that constructor cannot recover them ("Cannot extract string from
SearchValues"), so it leaves CanSeek = false and ExpectedChars empty. Terms.AnyOf
was therefore excluded from the seeking optimizations that Literals.AnyOf and
Terms.NoneOf get, for every consumer on net8.0+.

Construct SearchValuesCharLiteral from the span directly, which both wraps once
and keeps the chars.

Two tests lock this in: one asserts every Terms.* factory wraps in exactly one
SkipWhiteSpace, the other that AnyOf is seekable. Both fail before this change.

A benchmark comparing the two shapes is added to AnyOfPatternBenchmarks. The
direction is consistent (7 of 8 measurements favour the single wrap, ratios
1.02-1.40) but the magnitude is within noise on my machine, so the argument for
this change is the lost seekability and the divergence from every sibling
factory, not the layer cost.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The constructor tested the fields `_minSize` and `_negate` before assigning them
from the parameters of the same name, so the condition was always
`0 > 0 && !false` -- false. CanSeek stayed false and ExpectedChars stayed empty
for every ListOfChars instance ever built, meaning Literals.AnyOf and
Terms.AnyOf silently opted out of the seeking optimizations on exactly the two
target frameworks that need the help most.

The net8.0+ implementation in SearchValuesCharLiteral assigns first and tests
the parameters, and is correct; this brings ListOfChars in line.

Found by the AnyOfShouldBeSeekable test added in the previous commit, on the
net472 leg introduced earlier in this branch -- the first defect that leg caught,
and one that had been unreachable by the test suite since the code was written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Character.IsDecimalDigit and Character.IsHexDigit had two implementations: a
SearchValues<char>.Contains probe on net8.0+, and IsInRange / HexConverter.IsHexChar
downlevel. CharacterSetsBenchmarks already documented that SearchValues.Contains
beats the 64 KB _characterData table lookup, but it had never been compared against
the BCL's char.IsAscii* predicates, which are pure arithmetic and touch no memory.

Measured (MediumRun, 15 iterations x 2 launches):

  SearchValuesContains_IsDecimalDigit  0.191 / 0.195 ns
  IsAsciiDigit                         0.016 / 0.014 ns
  SearchValuesContains_IsHexDigit      0.116 / 0.215 ns
  IsAsciiHexDigit                      0.000 / 0.007 ns

An order of magnitude, so both predicates move to Character.cs as unconditional
char.IsAsciiDigit / char.IsAsciiHexDigit calls, with CharPolyfills backfilling the
two .NET 7 members downlevel over the exact expressions the #else branches used
before. Character.Mask.cs and Character.SearchValues.cs shrink accordingly.

DecimalDigits is "0123456789" and HexDigits is "0123456789abcdefABCDEF", so the BCL
predicates are exact equivalents rather than approximations. Two new tests assert
that over all 65536 char values, against the same string constants the SearchValues
instances are built from -- which also exercises the polyfill on the net472 leg.

IsIdentifierStart and IsIdentifierPart are unchanged: they have no BCL equivalent,
and the benchmark in this file shows SearchValues.Contains is the best option for
those sets. _decimalDigits and _hexDigits stay too -- Scanner uses them for span-wide
IndexOfAnyExcept, which is what SearchValues is actually for.

This commit changes net8.0/net10.0 codegen; the numbers above are the gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sebastienros
sebastienros merged commit e7085ee into sebastienros:main Aug 3, 2026
2 checks passed
@lahma
lahma deleted the perf/polyfill-consolidation branch August 3, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants