The bool-plus-out-abortTransform contract encoded three outcomes in two
flags, so false meant either 'sequence ended' or 'reject the transform'
depending on the flag. A three-value enum names each outcome at the
return site, and passing minExpectedOffset by ref makes visible that
only the binary.add path advances the expected offset (the bare ldloc
path previously echoed it back through an out parameter).
Assisted-by: Claude:claude-fable-5:Claude Code
HandleCpblkInitializer already rejects fields with a nil metadata token
before casting to FieldDefinitionHandle; the localloc prefix path did
not, so a crafted assembly could make GetFieldDefinition throw instead
of the transform being skipped.
Assisted-by: Claude:claude-fable-5:Claude Code
The jagged-array branch of DoTransform still assembled the
Block(ArrayInitializer) shape by hand; mapping its sequential values to
indexed tuples lets it use BuildSimpleArrayInitializerBlock like the
other single-dim branches, removing the last inline copy of the pattern.
Assisted-by: Claude:claude-fable-5:Claude Code
A true result meant both 'consumed a prefix' and 'no prefix present',
while false aborted the whole transform. The distinction is unnecessary:
a malformed or absent prefix leaves pos unchanged, so the per-element
stobj scan fails on the initblk/cpblk instruction and rejects the
transform with the same out-state. Aborting on 'no prefix' had also
broken plain constant-length stackalloc initializers (element stores
without any prefix), caught by CS73_StackAllocInitializers.
Assisted-by: Claude:claude-fable-5:Claude Code
HandleSequentialLocAllocInitializer mixed three concerns in one method
body: consuming an optional initblk/cpblk prefix, matching the offset of
each sequential store, and assembling the values array. Moving the first
two into TryHandleLocAllocInitializerPrefix and TryGetSequentialStoreOffset
reduces the method to the scan loop itself. Behavior-preserving; the
break-vs-abort distinction of the offset matcher is kept via an out flag.
Share the repeated block construction used by single-dimensional and multi-dimensional simple array initializers. The helper also avoids the LINQ iterator used for adding initializer stores while keeping the transformation behavior unchanged.
Assisted-by: OpenCode:openai/gpt-5.5:OpenCode
WebCilFile builds raw native pointers into the memory-mapped view directly
from section-header fields read out of attacker-controlled metadata. Unlike
PEFile.GetSectionData, which delegates to the bounds-checked PEReader, this
hand-rolled path validated nothing: a crafted section header could produce a
SectionData (and hence a BlobReader) pointing far outside the view, an
out-of-bounds read reachable on normal decompilation through method-body and
field-data RVA resolution. The (int)RawDataSize narrowing cast could also
yield a negative length.
Resolve and bounds-check the raw-data range against the view length before
constructing SectionData, widening the arithmetic to long so crafted uint
fields cannot wrap the range check or narrow into an apparently valid length.
Structural parsing in FromFile now reports a crafted or truncated module as
"not a WebCIL file" (null) rather than letting EndOfStreamException,
OverflowException or BadImageFormatException escape the loader.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Backfills the standard MIT X11 header on hand-written files that never
got one, attributing each to its first-commit author and year from git
history. Code vendored from dotnet/runtime and Humanizr/Humanizer gets
its origin's license lines and a provenance note instead. Generated
files (Resources.Designer.cs, the version-info template), tool-managed
suppression files, and BAML test-case fixtures intentionally stay
header-less.
Assisted-by: Claude:claude-fable-5:Claude Code
The C# compiler lowers a foreach over an inline array into a for loop whose
body reads each element through <PrivateImplementationDetails>.InlineArrayElementRef(ref
buffer, i). That helper cannot be named in C#, so the decompiled for loop did
not compile.
Reconstruct the foreach at the AST level instead. Rewriting the unchecked
InlineArrayElementRef helper to the bounds-checked indexer buffer[i] would be
unsound for an out-of-bounds index, so the transform fires only when the loop
bound equals the inline array's length: that proves 0 <= i < length, matching
the exact shape the compiler emits and nothing else. A loop that does not match
keeps the faithful (if unnameable) helper call rather than silently gaining a
bounds check.
Assisted-by: Claude:claude-fable-5:Claude Code
A value-type constructor chains via 'this = new TSelf(...)', an ordinary
body statement, so a hoisted argument null-guard in front of it is legal
C# output as-is; folding it back only bought lifting the chain into a
this(...) initializer. That cosmetic gain does not justify the extra
stobj shape matching, so the guard now stays in the body for structs and
the gate reduces to the ChainedConstructorCallILOffset check.
Assisted-by: Claude:claude-fable-5:Claude Code
The loop-shape decision can use ILVariable use lists before AST lowering, so avoid creating a for-loop when its iterator updates a byref local that must remain usable after the loop.
Assisted-by: OpenAI:openai/gpt-5.5:OpenCode
Per @siegfriedpammer: rather than rescue the hoisted for-initializer in
DeclareVariables, don't form the for-loop at all. TransformFor now bails when the
loop variable is a by-ref-like local used after the loop, leaving the while-loop
(matching source); the ref decl keeps its initializer, no CS8174. Reverts the
DeclareVariables change; test now expects the while form.
Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
A ref local that is used after a for-loop has its declaration hoisted in front of
the loop, but its only initialization is the for-initializer ref-assignment. The
declaration was then emitted without an initializer (`ref T x;`), which does not
compile (CS8174).
When a by-ref-like local's matching assignment is the first for-initializer, move
the ref-assignment's value up into the declaration (`ref T x = ref expr;`) and
drop the for-initializer.
Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
A type's leading field assignments are extracted to field declarations
only when every constructor that does not chain with this() agrees on
them. When they disagree, the analyzer gave up on the whole type, so the
remaining constructors' this()/base() calls were never lifted into
initializers -- a struct with two divergent constructors plus a chaining
one rendered the chain as `this = new TSelf(...)` instead of `: this(...)`.
Chain lifting does not depend on the shared-initializer extraction, so a
mismatch now just skips the extraction (the assignments stay in the
bodies) and the transform continues. Primary constructors still bail,
since their parameters drive the initializers that must be extracted.
Assisted-by: Claude:claude-opus-4-8:Claude Code
When a chained constructor-call argument contains a throwing null-check
(e.g. `value?.Length ?? throw ...`) and the argument is evaluated more
than once, the compiler hoists the null-check in front of the chained
call. The hoisted `if (value == null) throw ...;` then became the first
body statement, so MoveConstructorInitializer could not recognize the
chained call and left it as an illegal in-body `base..ctor(...)` /
`this..ctor(...)` (a parse error).
Fix it in the ILAst, where the `?? throw` shape already lives, rather
than re-deriving it on the C# AST: NullCoalescingTransform folds a guard
that directly precedes the chained call back into the first use of the
parameter as `if.notnull(ldloc param, throw)`. Nothing in a constructor
body can legally run before the chained call, so a statement preceding it
is necessarily compiler-hoisted; matching is by ILVariable identity, not
parameter name. The guard disappears before the AST transforms run, so
they need no change.
Reference types chain via a base/this..ctor CallInstruction; value types
chain via `this = new TSelf(...)`, i.e. stobj(ldthis, newobj TSelf(...)),
which ChainedConstructorCallILOffset does not report -- so the value-type
chain (including the case where this is spilled to a stack slot because
the guard sits between its load and the call) is matched directly.
Assisted-by: Claude:claude-opus-4-8:Claude Code
LocalFunctionMethod.MemberDefinition returned the instance itself, so the
declaration of a generic local function (whose base method is an identity
specialization) and its use sites (whose base methods carry the use-site
substitutions) never compared equal, and click-highlighting could not group
them. Returning a wrapper around the unspecialized base method lets the
token writer record the same definition object for the declaration and all
use sites. Method-group references to local functions additionally need
their own lookup, because GetSymbol() does not surface a
MethodGroupResolveResult.
The token-writer tests now keep the PEFile alive for the duration of each
test: recorded references are type-system entities that lazily read from
the PE image, and formatting one in an assertion message after disposal
crashed with an AccessViolationException.
Assisted-by: Claude:claude-fable-5:Claude Code
For an async iterator with an [EnumeratorCancellation] cancellation token, the
hoisted-local cleanup (stfld <>u__N(this, null)) can be emitted before the
combined CancellationTokenSource disposal in the set-result and catch blocks.
CheckSetResultReturnBlock and ValidateCatchBlock only consumed that cleanup after
the disposal, so the `pos + 2 == count` test missed the dispose pattern and the
analysis failed, leaving the raw state machine (catch (object), goto case, ...).
Allow the cleanup to appear before the combined-tokens disposal as well.
Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
Doc-comment folds were created with defaultCollapsed hardcoded to true,
so /// blocks always started collapsed. The new Display option is
bridged into DecompilerSettings like the other expand flags and drives
the fold's default state.
Detecting the last line of a doc-comment block requires sibling
navigation between trivia nodes, so trivia now carry parent, sibling
list, and index state. That state lives on the Trivia subclass to keep
AstNode's size unchanged, and the tree API treats trivia as a separate
navigation space: Next/PrevSibling and Remove work within the owning
list, Slot is null, ReplaceWith throws instead of aliasing the trivia
index into the parent's child slots, GetNextNode/GetPrevNode stop at
the list boundary, Clone and CopyAnnotationsFrom deep-copy and reparent
trivia, and CheckInvariant validates the trivia lists.
Assisted-by: Claude:claude-fable-5:Claude Code
Avalonia apps pack every resource (compiled XAML, image/SVG assets, ...)
behind a single !AvaloniaResources manifest blob, which until now fell
through to the generic resource node and could not be browsed. Parse the
blob's index and expose each packed file as its own entry, mirroring how
.resources files are unpacked, so individual files can be viewed and
saved. The reader is bounds-checked against crafted offsets/sizes in the
same defensive spirit as the recent .rsrc parsing guards.
Assisted-by: Claude:claude-opus-4-8:Claude Code
The step recorder is shared by C# AST and ILAst replay, so keep the public infrastructure out of IL.Transforms and pass language-specific node navigation through neutral node metadata.
Assisted-by: OpenCode:openai/gpt-5.5:OpenCode
The IL (Stepper) and C# (TransformContext) paths each recorded the changed
node, its seam neighbours, and its ancestor chain with duplicated logic that
could drift. Move the ordering/dedup/seam strategy onto
Stepper.Node.RecordModifiedNode; each language keeps only its own node
navigation.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Keep the IsUnsafe visitor path context-free while avoiding repeated null guards around debug-step recording in the unsafe modifier transform.
Assisted-by: OpenCode:openai/gpt-5.5:OpenCode
Keep existing modified-node candidates when a transform cannot provide a produced node, matching the IL transform helper and preserving highlight fallback quality.
Assisted-by: OpenCode:openai/gpt-5.5:OpenCode
When the step limit falls on a C# transform step, Stepper.Step records the node
as LimitReachedStep but throws before TransformContext can attach the node's
highlight candidates, so the 'show state before' view had only the bare modified
node to resolve against -- and nothing if that node renders no text of its own.
The IL path already records its candidates before the throw; mirror that on the
C# side by attaching the candidates to the limit-reached node in the catch, then
re-throwing so the pipeline still halts.
Assisted-by: Claude:claude-opus-4-8:Claude Code
NodeLookup.AddNode indexed every annotation of every rendered node by reference
identity, but the debug-step highlighter only ever looks up the DebugStepMarker;
the rest were dead keys, and a shared annotation (ResolveResult and friends,
copied across nodes) would resolve to whichever node rendered last. Make
DebugStepMarker public and bridge only it -- behaviour-preserving for resolution
while dropping the per-annotation dictionary churn on every rendered node.
Assisted-by: Claude:claude-opus-4-8:Claude Code
The debug-step node bracket (MarkNodeStart/try/finally/MarkNodeEnd) was copied
into every hand-written WriteTo override and re-emitted by the T4 generator, so
a newly added instruction could silently omit it and lose step highlighting with
no compile error or test failure. Seal WriteTo on ILInstruction to apply the
bracket once and delegate to a new abstract WriteToCore; move every override
(hand-written and generated) to WriteToCore without the wrapper. Rendered output
is unchanged -- the marks are no-ops unless the output tracks nodes.
Assisted-by: Claude:claude-opus-4-8:Claude Code
A step that removes a node has nothing left to highlight in the resulting text,
so range resolution fell back to the enclosing block and flooded it. Record the
changed node's surviving neighbours as seam anchors (captured before the
mutation) and split a step's candidates into precise / seam / ancestor tiers:
when neither the node nor its marker resolves, place a zero-length caret at the
gap -- the successor's start, else the predecessor's end -- and only fall back
to the enclosing block when no neighbour survives. A zero-length highlight is
rendered as a caret (positioned, pulsed, centered) with no background mark.
Assisted-by: Claude:claude-opus-4-8:Claude Code
The C# debug-steps view highlights and centers the exact AST node a
transform changed; the ILAst view already had the step tree and
replay-at-step but produced no highlight. Bring it to parity.
IL rendering has no token-writer seam like the C# output visitor, so
per-instruction text spans are recorded by bracketing
ILInstruction.WriteTo via a new INodeTrackingOutput. The dominant
inst.ReplaceWith(newInst) transform pattern detaches the instruction
passed to Step, so ILTransformContext gains EndStep to record the
produced instruction; Stepper additionally records the position's
ancestor chain as fallback candidates before the step-limit throw, so
the "show state before" view -- which halts at the selected step --
still resolves to a surviving ancestor (ultimately the ILFunction).
The highlight-range resolver is shared with the C# language.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Record AST transform groups and mutation steps through the C# pipeline, replay selected steps with the stepper, and carry modified-node ranges through output so the Debug Steps pane can highlight the selected mutation without replacing its full step tree.
Assisted-by: CodeAlta:gpt-5.5:CodeAlta
A field, auto-property, or event initializer is written once at its
declaration, but in IL it runs in every instance constructor that does not
chain to this(...) (and static initializers run in the static constructor).
The decompiler lifts the initializer from a single constructor, so its
breakpoint was emitted only there and the other constructors had none.
Two causes are addressed:
- The lift discarded the initializer's copies in the other constructors.
They are now kept on MemberInitializerInOtherConstructorsAnnotation and
replayed by SequencePointBuilder, mapping the same source location onto
each constructor's IL.
- PortablePdbWriter only emitted methods that DebugInfoGenerator discovered
through declaration syntax, so a constructor whose declaration is omitted
from the output (implicit default ctor, implicit static ctor, primary
ctor) dropped its generated points. Those functions are now emitted by
walking the sequence-point map directly.
PdbGen fixtures cover single, multiple, this()-chained, implicit, static,
primary-constructor, and field-like event initializers, pinning the
reconstructed breakpoint map against the C# compiler's.
Assisted-by: Claude:claude-opus-4-8:Claude Code
ReadWin32Resources walked the PE resource directory tree with raw native pointer
arithmetic over attacker-controlled offsets, counts and sizes, with no bounds
checks, no recursion-depth limit and no cycle detection. The root section pointer
came from GetSectionData, whose length was read and then discarded, leaving every
dereference unbounded.
A crafted assembly could therefore turn merely opening it (the Save as project
feature reads these resources unconditionally) into an uncatchable process kill or
an out-of-bounds native read: a subdirectory entry pointing back at itself recursed
until the stack overflowed; an inflated entry count walked off the section end; and
a data entry whose Size was up to 4 GB made Buffer.MemoryCopy read far past the
section, faulting on an unmapped page or copying adjacent process memory into the
byte[] later written to app.ico/app.manifest on disk. None of this is containable,
since a StackOverflowException cannot be caught and the repo has no corrupted-state
exception handling. This is the sibling of the bundle signature fix in a154a7bbb.
Carry the section length alongside the root pointer and bounds-check every offset,
entry count, name-string length and data Size against it, cap recursion depth and
track visited directory offsets to break cycles. A hostile or truncated file now
yields a bounded, partial tree instead of a crash; well-formed resources parse
exactly as before. The parser no longer needs the whole PEReader, only a delegate
that resolves a data RVA to a bounded pointer, which is the seam the new tests drive
over a pinned buffer.
Assisted-by: Claude:claude-opus-4-8:Claude Code
SingleFileBundle.IsBundle scans for the 32-byte bundle signature and reads the
8-byte header offset stored immediately before it. That offset only exists in a
genuine bundle, where the signature sits near the end of the file. The scan
started at the first byte, so a crafted file with the signature at offset 0..7
made it read before the start of the buffer. On the production path that buffer
is a page-aligned memory-mapped view, so the read faults on the preceding
unmapped page with an AccessViolationException -- a corrupted-state exception
that bypasses the loader's catch and terminates the process merely from opening
the file. The bundle probe runs on every opened file, so this needs no user
action beyond open.
Skip the backward read unless the match is at least sizeof(long) bytes into the
buffer. While in the same file, bound ReadManifest's FileCount against the bytes
that remain before it pre-sizes the entry array, so a crafted manifest can no
longer request a multi-gigabyte allocation.
Assisted-by: Claude:claude-opus-4-8:Claude Code
ILFunction.IsAsync is derived from the method signature, so .NET 11
runtime-async methods (MethodImplAsync bit, no MoveNext state machine)
report IsAsync without AsyncAwaitDecompiler ever populating
AsyncDebugInfo. Its Awaits then stays an uninitialized ImmutableArray,
and PortablePdbWriter threw an NRE building the MethodSteppingInformation
blob from that default struct. Runtime-async methods have no yield/resume
offsets to record, so guard on Awaits.IsDefault and omit the blob,
matching the C# compiler, which emits no stepping information for them
either. A genuinely zero-await classic state machine keeps an
initialized empty Awaits and is unaffected.
Assisted-by: Claude:claude-opus-4-8:Claude Code
DebugInfoGenerator asserted that every local variable's decompiled type
is equivalent to the metadata local-signature type at its slot. That
holds at IL-read time but not afterwards: variable splitting gives one
slot several typed variables, pinned-region locals are modeled as
pointers (int* vs int& pinned), and generic-context type-parameter
identity differs. The assertion therefore aborted PDB generation (Debug
builds) for ordinary inputs such as any method with a fixed statement.
The type is never written to the PDB - only the slot index and name are -
so the mismatch cannot affect the debugging experience. The slot index,
the only emitted value, is correct by construction: it is the IL
ldloc/stloc operand, sourced from the signature slot when the variable is
created and copied verbatim by SplitVariables; it is never reassigned.
Keep only the index-bound check and document why the type is not verified.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Decompiler warnings (ILFunction.Warnings, e.g. the DetectPinnedRegions
block-duplication notice) are surfaced as an EmptyStatement carrying only
a comment. VisitEmptyStatement prints no semicolon for it, and
EmptyStatement derives its StartLocation/EndLocation from its Location
field, which is only set when that semicolon token is written. The
statement was therefore left without a text location, and
SequencePointBuilder then asserted on the empty start location while
generating PDB sequence points - aborting PDB generation for any assembly
whose decompilation emits such a warning (e.g. System.Net.Requests).
Point the empty statement at the comment it carries (already printed by
the time the node ends, so its location is known), falling back to the
collapsed end-of-last-token position. Every printed node then has a
location, so the SequencePointBuilder invariant holds without
special-casing, and the statement lines up with the text it represents.
Assisted-by: Claude:claude-opus-4-8:Claude Code
VisitDynamicUnaryOperatorInstruction handled every dynamic unary operator
except ExpressionType.OnesComplement, so ~x on a dynamic operand fell through
to the unsupported-opcode error expression and produced uncompilable output
(an incomplete cast that fails to parse). Map it to the bitwise-complement
operator, like the sibling unary cases.
Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
After inlining turns a runtime accumulator (e.g. a hand-written GetHashCode prime
chain `h = h * -1521134295 + ...`) into one expression, the leading `SEED * PRIME`
becomes a compile-time constant subexpression. C# always evaluates constant
subexpressions in a checked context, so emitting it bare fails to compile with
CS0220, even though the surrounding context is unchecked.
HandleBinaryNumeric now annotates such an overflowing constant binary operation
with ExplicitUncheckedAnnotation (instead of the implicit UncheckedAnnotation), so
AddCheckedBlocks wraps it in an explicit unchecked(...) - mirroring the existing
handling of overflowing constant n(u)int casts.
Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
NullPropagationTransform rewrote `c != null ? c.AccessChain : default` to
`c?.AccessChain ?? default` whenever the access-chain result was a non-nullable
value type. For a by-ref-like type (a ref struct such as Span<T>) that form does
not compile: a ref struct cannot be wrapped in Nullable<T> (CS8978). Exclude
by-ref-like return types from the null-coalescing rewrite.
Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI
F# emits tail calls pervasively, but the 'tail.' prefix was dropped
entirely at the C# output stage, so the information never reached the
decompiled text. Render it inline as '/*tail.*/' before the call,
mirroring the existing 'constrained.' prefix comment in CallBuilder.
Fix#3817
Assisted-by: Claude:claude-opus-4-8:Claude Code
The project listed all 576 source files as explicit <Compile Include> with
EnableDefaultItems=false, so every new file had to be added by hand. Switch to
the SDK's default **/*.cs glob and keep only the exclusions that are actually
needed:
- Properties/DecompilerVersionInfo.cs is generated before build and absent when
the glob is evaluated on a clean checkout, so it is still included explicitly
(with a paired Remove to avoid a duplicate once a prior build produced it).
- DecompilerVersionInfo.template.cs is a placeholder template, never compiled.
Also delete the hand-written DecompilerAstNodeAttribute.cs: the source generator
emits this attribute (RegisterPostInitializationOutput), so the on-disk copy was
stale dead code -- never compiled, and parameterless where real usages pass
[DecompilerAstNode(hasPatternPlaceholder: true)]. Removing it drops what would
otherwise be a third exclusion.
Default None globbing stays off so the explicit None entries remain the
authoritative list. The evaluated Compile set is identical to the previous 576
files; verified by a clean build with the version file both present and absent.
Assisted-by: Claude:claude-opus-4-8:Claude Code
A stackalloc whose result is a pointer is only valid C# as the initializer of a
pointer-typed local. The inliner moved a single-use pointer stackalloc into its
use, producing e.g. 'K.V(stackalloc int[3] { 1, 2, v })' or '*stackalloc ...';
in an expression position the stackalloc is typed as Span<T>, which does not
convert to a pointer, so the output did not compile. Keep such a stackalloc as a
separate local. Moving it into a local store (its declaration) and into the
Span<T>/ReadOnlySpan<T> constructor stay allowed, since those are the positions
where the pointer or span form is exactly what is wanted.
Found while exploring stackalloc-initializer coverage.
Assisted-by: Claude:claude-opus-4-8:Claude Code
HandleSequentialLocAllocInitializer formed a stackalloc initializer whenever the
explicit stores were contiguous from offset 0, even if they did not cover the
whole buffer. A 'stackalloc byte[16]' reinterpreted as int and written through
three of its four elements decompiled to 'stackalloc int[4] { 1, v, 3 }', whose
initializer has fewer elements than the declared length and does not compile.
Require every element to be written (from the constant data blob or an explicit
store) before forming the initializer; otherwise the buffer stays a plain
stackalloc with individual stores.
Found while exploring stackalloc-initializer coverage.
Assisted-by: Claude:claude-opus-4-8:Claude Code
A constant-size stackalloc initializer stores its constant elements through a
data blob (cpblk from a <PrivateImplementationDetails> field). ReadElement
decoded every element by width, so a 4-byte float was read as Int32 and an
8-byte double as Int64. The resulting constant carried the raw bit pattern
(1f decoded as 1.0653532E+09f) and its stack type no longer matched the store,
tripping StObj.CheckInvariant. Dispatch on the element's type code so Single and
Double are read as floating-point, matching the heap-array decoder.
Found while exploring stackalloc-initializer coverage around the element-type
hint fix; floating-point element types had no test case.
Assisted-by: Claude:claude-opus-4-8:Claude Code
A constant-size stackalloc initializer whose buffer is only passed to a static
call (never dereferenced as a typed pointer) threw "given Block is invalid!".
TranslateStackAllocInitializer recovers the element type from the surrounding
type hint, but that hint is unreliable for this shape: the constant allocation
size is folded to a byte count, so it can no longer be read off a
'count * sizeof(T)' expression, and the buffer is kept on the stack as a native
int instead of a T* local, leaving the hint a non-pointer. The guard only
repaired an incompatible pointer hint, so a non-pointer hint fell through to a
'byte' element type that is incompatible with the actual stores, and the
per-element check threw. Derive the element type from the type being stored
whenever the hint is not already a compatible pointer.
The regression is driven by the code shape, not the compiler version: in
optimized builds the buffer is kept on the stack as a native int whenever it is
passed to a static call without being dereferenced. This reproduces across
Roslyn versions, including the pinned test compiler, so the added fixture is red
without this fix in the optimized configurations.
Assisted-by: Claude:claude-opus-4-8:Claude Code
After the Role and TokenRole hierarchies were removed, the Roles class no
longer held any roles -- only the punctuation and keyword text the output
visitor writes (LPar, Arrow, ClassKeyword, ...). The name was a misleading
vestige of the deleted system. Rename it to Tokens, which is what it now
is; the constants and their values are unchanged.
Also refresh the CSharpSlotInfo doc, which still described slot identity as
the successor to the removed node.Role == Roles.X comparison.
Assisted-by: Claude:claude-opus-4-8:Claude Code
A slot kind names one child position, so the generator emits a single
typed Slots.X constant for it. A kind used with two different child types
had to widen that constant to AstNode, and the typed GetChildren<T>
accessor then cast the live AstNodeCollection<Concrete> to
AstNodeCollection<AstNode> -- an InvalidCastException waiting for the
first caller (latent today, but the model permitted it).
Make the loose state unrepresentable instead of guarding it at runtime:
DSTG001 errors when a [Slot] kind is declared with more than one child
type. The six kinds that only coincidentally shared a name (Body, True,
False, Initializer, SwitchSection, Variable) are split into precisely
typed kinds; a genuinely either/or position (a lambda body) stays one
declared type, AstNode. Every Slots.X now carries its real element type,
so the GetChildren cast can no longer fail.
Output is unchanged: the Pretty suite stays byte-identical with
CheckInvariant green in DEBUG.
Assisted-by: Claude:claude-opus-4-8:Claude Code
These helpers are documented to return null when nothing matches, but
returned `null!`, hiding the very nullable warnings #nullable enable is
meant to surface: a miss handed back null typed as non-null and would
NRE downstream with no compile-time signal. Returning T? lets the
compiler enforce the guard at each call site (both existing callers
already null-check). GetVariable forwards the result, so it becomes
VariableInitializer? to match.
Assisted-by: Claude:claude-opus-4-8:Claude Code
A single slot always occupies the same flattened index, so filling,
clearing, or replacing it moves no other child -- only the new child's own
index needs setting. The generated single-slot setters now pass that index
to SetChildNode (a compile-time constant when no collection precedes the
slot; SetChild forwards its argument), which assigns childIndex directly and
never invalidates -- mirroring the IL AST's SetChildInstruction(ref, value,
index). Previously a set-from-null could not know the index here and fell
back to invalidating, forcing a later O(children) EnsureChildIndices rebuild.
With the indices now kept current by construction, NextSibling/PrevSibling
inline the validity check and skip the (non-inlinable) EnsureChildIndices
call in the overwhelmingly common already-valid case.
Together these idle the renumber machinery on a System.Private.CoreLib
decompile: EnsureChildIndices calls 53.4M -> 1.6M, actual rebuilds
1.97M -> 183K, elements renumbered 3.40M -> 757K. Output is byte-identical
and the Pretty suite stays green with CheckInvariant validating the
directly-assigned indices after every transform.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Removing a collection element invalidated the parent's whole flattened
index set, so the next sibling navigation rebuilt it in O(children) -- and
that renumber, not the array shift, was the dominant cost: a reverse
(tail-first) removal, which shifts nothing, was still quadratic, isolating
EnsureChildIndices as the culprit.
When a node's only collection is also its last slot it owns the contiguous
range [base, base + Count) with nothing after it, so an element's flattened
index is just base + its local position (base is the slot's declaration
position, since the preceding slots are all single children). On that
fast-path -- which the generator now flags, passing the base index -- Add
indexes only the appended element, Insert/Remove renumber only the shifted
suffix, and IndexOf is base-relative O(1); none of them invalidate. Other
shapes (several collections, or a slot after the collection) keep the
invalidate-and-rebuild fallback. Tail and scattered removal, and removal
during traversal, no longer pay the per-operation renumber.
Microbenchmark, removing every element of an N-element block: tail-first at
N=32000 went 1493 ms -> 0.5 ms (now O(N)); front-first is ~1.6x faster and
no longer renumbers (its residual cost is the inherent array shift). Output
is byte-identical and the Pretty suite stays green with CheckInvariant
validating the maintained indices after every transform.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Siegfried Pammer
Christoph Wille
Sebastien Lebreton
nikitalita