When an IfInstruction was created with resultType=bool, because only
values 0 or 1 are possible, then we need to preserve this property in
transforms -- otherwise the added correctness test would fail.
The decompiler emits comments - //IL_ warnings, "Could not convert
BlockContainer", "try-fault", a Nop's comment - as an EmptyStatement in the
middle of a statement sequence. Every transform that walks such a sequence
then stops recognizing its pattern the moment one of those lands in it:
constructor initializers stay in the body, `using var` and `for` are not
introduced, and a finalizer keeps its `override Finalize` shape, which does
not compile at all.
The destructor matcher moves the placeholders it skipped into the body that
replaces the old one, so the warning that caused the problem is not dropped
along with the statement carrying it.
Assisted-by: Claude:claude-opus-5:Claude Code
A constructor store to an auto-property's backing field is expressible
after the field declaration is gone in one of two ways: ReplaceBackingFieldUsage
rewrites it to an assignment of a setter-less property, or
TransformFieldAndConstructorInitializers lifts it into a property initializer.
A deconstruction target assigns several members at once, so it can never take
the second route, and a property that kept a setter would invoke that setter
instead of storing the field. Without the restriction the declaration is
removed while the store keeps referencing it.
Assisted-by: Claude:claude-opus-5:Claude Code
The parameters standing in for the expanded arguments were built from
the element type of the array the compiler had built, not from the
element type the params collection declares. The two can only differ
where the collection is covariant in its element type, and no compiler
emits that shape today - it materializes the array into a local of the
target type first, which the pattern no longer matches - so this only
removes the dependency on that.
Where the params collection is one overload resolution cannot unpack,
the expanded form is now abandoned instead of being built from a type
that resolution would never have used.
Assisted-by: Claude:claude-opus-5:Claude Code
MatchArrayInitializerFinal selects the operator by its return type, so
the block's result type is that return type. The declaring type only
happens to be the same one for the two array-to-span operators; Span<T>
also declares the conversion to ReadOnlySpan<T>.
Assisted-by: Claude:claude-opus-5:Claude Code
The order of a record's fields and properties has to be known, because
Equals, GetHashCode, PrintMembers and the copy constructor are recognised
by walking their bodies in lockstep with it. It was assumed to be every
property followed by every field, so a record that declares a field
before a property desynchronised all four at once: none was recognised as
generated, all of them were emitted, and the auto-properties lost their
backing fields to raw <Property>k__BackingField accesses - output that
does not compile.
The order is in the generated members themselves, but no single one has
all of it: Equals compares everything that carries state and never a
computed property, PrintMembers prints everything public and never a
private field. Both follow declaration order, so the two sequences are
merged along the members they share, which puts a private field and a
computed property back in the right places relative to each other.
Members neither of them mentions - EqualityContract, static members -
keep the position they had.
Where the two orders conflict, which can only happen for a member that
one of them never sees, the equality order wins; nothing in the metadata
says more, and the choice cannot change more than the order the members
are printed in.
Assisted-by: Claude:claude-opus-5:Claude Code
An array initializer standing in for a Span<T>/ReadOnlySpan<T> reaches the
call builder as an implicit span conversion over the array creation, a shape
the params expansion did not know, so a params span argument came out as the
array the compiler had built rather than as the argument list that was
written.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
The block addresses its allocation through the pointer localloc returns, and
only its result is a Span<T>. Retyping the initializer variable to the span
made every element store ask for a pointer it no longer had, which was papered
over with a conv from the span; once Obj and VT became distinct stack types
that conv had no conversion kind left and the whole method failed to decompile.
The span constructor becomes the block's final instruction instead, so the
element stores keep the pointer they were written against and the block still
evaluates to the span.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
The Span<T>/ReadOnlySpan<T> initializer patterns replaced their call with an
array initializer block, so an array stood where a span was expected: the
enclosing leave and any call taking the result saw StackType.Obj against the
StackType.VT the span type demands. The block now ends in the implicit
conversion the C# compiler applies, which is the one shape besides a bare
ldloc that an array initializer may take; the expression builder keeps the
conversion out of the output but not out of the expression's type.
Naming that operator wants a method looked up by signature rather than by a
predicate over the type's members, so MetadataModule grows a ResolveMethod
overload for it, sharing its signature matching with the metadata path.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Metadata as attributes on an item element is MSBuild 15 syntax. The
non-SDK project format is what an export falls back to for toolchains
that predate the SDK, and those reject an unknown attribute on an item
element, so a Page item carrying Generator and SubType as attributes
undoes the reason to write that format at all. Every non-SDK project
written by anything else keeps metadata in child elements.
The SDK-style writer keeps attributes: there the syntax is a given and
it is what the format's own tooling produces.
Assisted-by: Claude:claude-opus-5:Claude Code
The project exporter wrote every XAML document to the project root under
a fully-qualified name while the code-behind class went into a directory
named after its namespace, so the two halves of one partial class ended
up in different places. WPF tooling pairs MainWindow.xaml with
MainWindow.xaml.cs by name and location; anything else is an unrelated
file to it, and --nested-directories made the split wider still by moving
only the C# half.
Both now go through one function that decides where a type's files live,
so the document lands where the type's own C# file would have, and the
code-behind is named after the document. The BAML writers of the UI and
of the command line had grown their own copies of the naming, which is
how they came to disagree with the C# writer in the first place.
Assisted-by: Claude:claude-opus-5:Claude Code
The exporter dropped PresentationFramework, System.Xaml, System.Windows.Forms
and System.Drawing from every project it wrote, whatever the assembly used,
while a second list held the remaining WPF assemblies behind a WPF check. The
SDK draws the line elsewhere: Microsoft.NET.Sdk.WindowsDesktop.props promotes
the nine _WpfCommonNetFxReference items to _SDKImplicitReference only when
UseWPF is set, System.Windows.Forms only when UseWindowsForms is, and
WindowsFormsIntegration only when both are. A XAML-only assembly therefore lost
a reference that nothing supplied, and a WPF application that also used Windows
Forms lost the Windows Forms references while the project only said UseWPF.
Following the SDK there makes WPF and Windows Forms independent rather than
alternatives, which is what the flags enum is for: an assembly can use both,
and then both properties have to be written. Where an assembly looks like more
than one kind of project, the web SDK wins the Sdk attribute, because
Microsoft.NET.Sdk.Web imports Microsoft.NET.Sdk and so carries the desktop
targets, while Microsoft.NET.Sdk.WindowsDesktop carries no web targets.
System.Drawing stays unconditional: Microsoft.NET.Sdk.BeforeCommon.targets adds
it for every .NETFramework target rather than only for Windows Forms ones, and
on .NET Core it ships in the Microsoft.NETCore.App reference pack.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
An exported WPF project listed PresentationCore next to the implicit
Windows Desktop framework reference, which is a duplicate reference
(MSB3243) or an unresolvable one (MSB3245) once the hint path stops
pointing anywhere. The target-pack filter that should have caught it
asks the assembly resolver, which answers by probing the shared
frameworks installed on the machine running the export - so the same
assembly exported from Linux, or from a Windows box without the
desktop runtime, produced a different project file. What the SDK adds
for UseWPF is a fixed list, so match it by name instead.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Microsoft.NET.Sdk imports the Windows Desktop targets itself for .NET
Framework and for .NET 5 and later, and warns (NETSDK1137) about every
project that still names the separate SDK. Only .NET Core 3.x, where
those targets are not imported without a platform-suffixed moniker,
genuinely needs Microsoft.NET.Sdk.WindowsDesktop.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
A .NET 5 or later project that sets UseWPF or UseWindowsForms is
rejected outright (NETSDK1136) unless its target framework names the
Windows platform, so an exported WPF assembly produced a project that
could not build at all. The platform belongs to the assembly rather
than to WPF - TargetPlatformAttribute records it, SupportedOSPlatform
its minimum version - so the moniker follows the attributes wherever
they are present, and falls back to plain "windows" only for a desktop
project built before those attributes existed. Monikers older than
net5.0 take no platform suffix and must not grow one.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
The WPF markup compiler generates the program entry point from the
ApplicationDefinition item, so an exported project that lists App.xaml
as a Page has no Main at all and fails to build with CS5001. Both the
UI and ilspycmd already resolve the BAML root's partial class, which
makes deriving Application from System.Windows.Application the natural
signal. The module additionally has to have an entry point of its own:
a library that merely contains an Application subclass would otherwise
have MSBuild generate a Main into it.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Sanitizing a resource name is not injective: "a+b/logo.png", "a&b/logo.png"
and "a#b/logo.png" all come out as "a-b/logo.png". The writers create files
with FileMode.Create, so every colliding entry but the last was lost, and
nothing was written to the error list to say so - an assembly can be built
to make that happen to as many entries as it likes. A WPF probe assembly
with 20 resource entries exported as 13 files.
Uniquifying is enough because the exported item already pins the true name
in its LogicalName, so the file on disk has to be unique, not faithful. The
suffix search resumes where the previous collision on a name left off, so a
crafted pile of collisions stays linear rather than quadratic, and the name
is trimmed to keep the segment within the file system's limit.
Directory creation moves inside the per-entry error recovery for the same
reason: an entry named after a directory another entry needs makes it throw,
and that has to cost the one entry rather than the rest of the container.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
A rebuilt WPF project only resolves its own pack URIs when every entry of
"<AssemblyName>.g.resources" comes back under the resource ID it had before.
The file on disk cannot carry that ID: it is sanitized for the file system,
and the ID itself is escaped. Verified against a WPF assembly built for this:
the WPF build tasks re-escape whatever LogicalName they are given, so the item
has to hand them the decoded name, and an entry left as EmbeddedResource
rebuilds into a manifest resource of its own instead of into ".g.resources".
The adjustment is made where the items are collected, so it covers the base
class and both hosts that plug their own BAML handling into it without
widening the WriteResourceToFile or IResourceFileHandler contracts.
The Resource build action this gives them is also what #2253 asks for.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
The SDK-style writer only ever emitted EmbeddedResource items, so every other
item type the export produced never reached the project file. XAML recovered
from BAML lands in Page items: ILSpy wrote the .xaml files to disk and the
project referenced none of them, leaving an exported WPF project that cannot
rebuild.
Explicit items collide with the SDK's own globs - a UseWPF project globs
**/*.xaml into Page, and NETSDK1022 rejects the duplicate - so each include is
preceded by a remove of the same item type, the pattern the EmbeddedResource
path already uses. Setting EnableDefaultPageItems=false would work as well, but
it is WPF-specific and switches off a glob for the whole project, whereas the
remove is per item, applies to any item type, and keeps the SDK's item
definitions (XamlRuntime) in effect.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
WPF's build tasks key every Page and Resource item in
"<AssemblyName>.g.resources" by the item's relative path, lower-cased and
run through Uri.GetComponents(Path, UriEscaped) - so a folder named
"Resource Test" arrives as "resource%20test", and a folder named in any
non-ASCII script arrives as a run of UTF-8 percent escapes. Those escapes
are not part of the name; sanitizing them turned "resource%20test" into
"resource-20test" and any Chinese or umlaut folder into a line of hex.
Measured against a WPF assembly built for this: only space, '#', '{', '}'
and non-ASCII bytes are ever escaped, and Uri.UnescapeDataString is the
exact inverse - anything the escaper leaves alone contains no percent
sign, and a literal one arrives as %25. Only the WPF-generated containers
are affected, so a percent sign in any other .resources file stays part
of the name.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Obfuscators put arbitrary characters into BAML strings, and XML 1.0 has no
representation for most control characters - a numeric character reference is invalid
for them too. Writing such a document threw ArgumentException from XmlWriter, which
loses the resource on project export and shows an exception instead of the page in the
UI. The escapes are spelled the way the C# output spells them, so one convention covers
both languages. Namespace URIs have to be escaped where the XNamespace is created rather
than in the final pass: the URI is baked into every element name built from it, so
patching only the xmlns declaration would desync the two. Characters XML can carry stay
untouched, so ordinary documents decompile byte-identically.
Every BAML stream of an assembly lives in one .resources container, and the recovery
around resource writing sat outside the loop over its entries, so a single page that
could not be written discarded every other page sharing the container with it.
Assisted-by: Claude:claude-opus-5:Claude Code
Change InferType() to an abstract method and implement for every ILInstruction.
With this change, we now always have enough information to create a variable of an appropriate type to store the result of evaluating the instruction.
This previously was not the case for instructions producing "other value type", for which the stacktype-based fallback incorrectly produced `object`.
C# 12 allows both on the explicitly typed parameter list of a lambda, and
nowhere else: an anonymous method cannot declare either, and neither can a
lambda whose parameter list is about to be dropped. Guarded by a setting so
the output stays valid for earlier language versions.
Only what the anonymous function's own metadata declares is written. A lambda
may state a default the delegate does not have, a different one, or none where
the delegate has one, and reflection over the lambda's method reports what the
lambda declared - so filling either in from the delegate's Invoke would make
the recompiled assembly describe itself differently from the original. Call
sites are unaffected either way, because they bind against the delegate, which
still declares both.
Roslyn writes ParamArrayAttribute on the anonymous function's own method only
from version 5 on; before that it stands on the delegate type alone, where it
is not the lambda's to restate, so the fixture guards those cases on ROSLYN5.
The correctness test reads the metadata back through reflection, which is the
only place the difference is observable.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
The C# 10 grammar only allows attributes on a lambda or its parameters
when the parameter list is parenthesized, but LambdaNeedsParenthesis
predates attribute support and only considered the single parameter's
type and modifiers. An attributed lambda whose parameter type is erased
for being anonymous therefore printed as '[My] a => a.X', which does not
parse. Latent since attributed-lambda decompilation was added: every
other attributed lambda has explicitly typed parameters, which already
force the parenthesized form.
Assisted-by: Claude:claude-fable-5:Claude Code
A missing value-type definition makes ILReader insert a Ref-to-Unknown conversion before instance calls. Preserve the managed-reference receiver so C# output does not fall back to invalid ref casts or unsafe pointers.
Assisted-by: Codex:gpt-5.6-sol:Codex
Assigning through a ref-conditional, (cond ? ref a : ref b) = value, was
emitted without the parentheses, so it re-parsed as
cond ? ref a : (ref b = value) and failed to compile (CS8156 / CS0201).
The target was only parenthesized above assignment precedence, but a
conditional binds tighter than assignment, so the check let it through.
Require the assignment target to have precedence above the conditional
operator. Ordinary lvalues (locals, fields, indexers) are primary
expressions and are unaffected; the postfix ++ form already parenthesized
correctly via unary precedence. Covers plain and compound assignments alike.
Assisted-by: Claude:claude-opus-4-8:Claude Code
Invariants that involve types (stack type of a variable against its IType,
the element type of an array access, the operand types of a comparison)
need a type system to resolve them against, and the only correct one is the
type system the instruction tree was decoded with. Until now CheckInvariant
took only the phase, so such a check had no compilation to use:
DeconstructInstruction.CheckInvariant called IsAssignment with a null type
system, which only held up because the targets it sees are ldloc, whose
InferType never touches the compilation; a ldflda-wrapped or pointer target
would have failed inside the invariant instead of reporting a violation.
Every call site already has that type system in scope: the ILReader's
compilation, the ILTransformContext of the running transform, or the
decompiler's own IDecompilerTypeSystem. It is now passed explicitly and the
base implementation asserts it is present, so a future invariant can rely
on it without re-plumbing the callers.
Assisted-by: Claude:claude-fable-5:Claude Code
This way, we don't need the MapToMergedBounds logic to split the merged list back into lower/upper.
Also, this commit avoids the quadratic merge-everything-with-everything else -- instead we use a dictionary to compare only types that are equivalent to begin with.
This is the same approach as Roslyn MethodTypeInference.Fix/AddAllCandidates.
NullPropagationTransform only rewrites "x != null ? x.Chain : fallback"
into "x?.Chain ?? fallback" when the chain's inferred type is a
non-nullable value type, and InferType had no case for ldlen. Array length
therefore came back as UnknownType, so "arr?.Length ?? 0" was left as a
ternary.
The inferred type mirrors ExpressionBuilder.VisitLdLen, which decides
between Array.Length and Array.LongLength from the result type alone.
Found while investigating #3704, where the surviving ternary also keeps the
tested array in a stack slot and strands the typeof of a dynamic call's
static target. That issue is fixed separately in #4072, whose DynamicTests
cases pinned the ternary as expected output; those blocks round-trip
exactly now, so they are gone.
Also carries a review follow-up that missed #4072: the static-target test
in VisitDynamicInvokeMemberInstruction is a plain null check, the way
DynamicInvokeMemberInstruction itself tests the field, rather than a
pattern match binding a name it does not need.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
Three places decided independently whether a backing field would still be
declared, and they disagreed. ReplaceBackingFieldUsage rewrote a constructor
store into a property assignment whenever the property looked collapsible,
without asking whether PatternStatementTransform would actually remove the
declaration; ConvertField printed the "field" keyword on the FieldKeyword
setting alone, ignoring the GetterOnlyAutomaticProperties veto that
MemberIsHidden applies to the same field.
Two consequences, both silent. A setter-less property under
GetterOnlyAutomaticProperties = false kept its declaration and got "field" in
the getter anyway, so the keyword bound to a second synthesized field and the
declared one went unwritten. A settable property under AutomaticProperties =
false had its initializing store turned into a property assignment that
TransformFieldAndConstructorInitializers could no longer lift, leaving the
constructor in the output with an unconverted base-constructor call.
BackingFieldWillBeRemoved is now the single verdict every branch consults, and
it mirrors the transform's own entry gate. A property that keeps explicit
accessors is no longer addressed by name: the store stays a field reference and
becomes the property initializer, which is what field-backed storage means. The
one exception is a setter-less property's constructor store, which C# allows to
be written as an assignment and which has no other expressible form.
IsBackingFieldOfAutomaticProperty now answers through TryGetBackingField instead
of its own name check, so the two directions of "is this field that property's
storage" cannot diverge on staticness or field type. ReplaceBackingFieldUsage
dispatches on the resolve result rather than the identifier's spelling; after
ConvertField the same field appears both as "field" and under its metadata name
carrying the same annotation, so matching the name was matching the wrong thing.
The keyword's own precondition moves into a CanUseFieldKeyword local function.
Seven clauses with comment blocks wedged between them had to be read as one
expression; as one early return per rule the list reads in order.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
ExpressionBuilder.ConvertField prints the C# 14 "field" keyword based on the
FieldKeyword setting alone, but PatternStatementTransform only entered the
property transform when AutomaticProperties was on. With FieldKeyword on and
AutomaticProperties off the backing-field declaration was therefore never
removed, and the output declared the backing field next to accessors already
written in terms of "field".
That output still compiles, which is what makes it dangerous: the keyword binds
to a second, freshly synthesized backing field while the declared one stays
unwritten, so the recompiled assembly has different storage than the input.
Only a CS0169 "field is never used" warning hints at it.
AutomaticProperties governs only whether trivial accessors collapse to
"get;"/"set;"; the declaration removal inside the transform is a separate step
that FieldKeyword alone is enough to justify. The entry gate now mirrors
CSharpDecompiler.MemberIsHidden, which already made the field's visibility
depend on either setting, with GetterOnlyAutomaticProperties vetoing the
getter-only case for both.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
HandleConditionalOperator collapses `if (c) a = x; else a = y;` into a
conditional operator, innermost first, and keeps going for as long as the
chain does. A source else-if ladder therefore comes back as one expression,
however long it was: the sample in #2027 decompiles to a single
2095-character statement, and one NLog method to 2230 characters nested 29
brackets deep.
ExpandNestedConditionals undoes that past one level, so a statement keeps at
most a single conditional operator.
It runs at the end of the pipeline rather than inside ExpressionTransforms,
because every transform that needs its input to be a single expression has
to see the collapsed form first: object and collection initializers, `with`,
switch expressions, interpolated string handlers, and the query lambdas the
C# stage later rewrites into clauses. Cutting the chain earlier leaves an
if-else between the statements they pattern-match on and they silently stop
matching - an object initializer assigning an init-only member then does not
even compile. The same reasoning ReduceNestingTransform gives for walking
back ConditionDetection's aggressive else-inlining once the structure is
settled.
A chain already stored to a variable is expanded into that variable, so
nothing has to be decided: the variable carries its own type. A chain in any
other position - an argument, a return value, a field store - has nothing to
expand into, and ILExtraction can give it one. The temporary ILExtraction
creates is typed from the stack type though, where `I4` is `int`, `bool`,
`char` and every enum at once, so extracting on that basis turned a bool
into `int num` with `if (num == 0)` and an enum into
`dbType = (IsFixedLength ? 22 : 0)`.
InferExpectedType is the counterpart to InferType that answers this: where
InferType asks what a value is, it asks what the position the value flows
into says it should be - a parameter, a return type, a field, all of which
carry their type in metadata. Extraction is done only where that question
has an answer, and the temporary is typed from it. The receiver of a call
then reads `XPathNavigator xPathNavigator`, not `object obj` with a cast
back, and a field store keeps its enum's member names.
The Pretty fixture covers what must NOT change: an array initializer, a
query lambda, a ref local, a switch expression, an object initializer with
an init-only member, a `with` expression, a catch-when filter and both
constructor-initializer forms. The positive case is an ILPretty test,
because a Pretty fixture is its own input and expected output, and a chain
that round-trips through collapse and expansion has no fixed point there.
The PdbGen test records the cost in breakpoints: the compiler's single
sequence point for the collapsed statement becomes one per expanded
statement, which is inherent to splitting a statement in two.
#2027
Assisted-by: Claude:claude-opus-5:Claude Code
A dynamic call site that names a type passes typeof(T) as its target. When
a later argument contains control flow, the compiler stores that typeof in
a temporary. ILInlining does not undo this: it inlines a store only into
the immediately following instruction, so an intervening statement leaves
the typeof behind, and the expression builder printed the temporary instead
of the type. Substituting the typeof back into the target slot is not an
option either, because a call there has side effects and blocks inlining of
the remaining arguments.
The type is therefore stored on the instruction. Object creation already
did this, but kept the field private, so the expression builder re-derived
the type from the target argument and failed on the spilled form. Member
invocation gets the same field. Both drop the target from Arguments, so the
dead-argument handling removes the store; ArgumentInfo keeps its entry for
the target, because the invocation symbol is built from it.
The type of an object creation is not nullable: the transform returns
before constructing the instruction when it cannot match the typeof, and
substitutes the unknown type otherwise. An unresolvable type therefore
reaches the expression builder as the unresolved type it is, and prints as
`?` like any other, rather than being indistinguishable from a missing one.
Those two are also the only binder method kinds that ever see
CSharpArgumentInfoFlags.IsStaticType. Every other way of naming a type as
the receiver binds statically and leaves only a conversion of the dynamic
operand behind.
Assisted-by: Claude:claude-opus-5[1m]:Claude Code
This was due to StackType.O doing double-duty as `object` and `other`.
While ExpressionBuilder would often improve the type of such locals, the `object` nevertheless ended up used in a couple of places, e.g. via the `typeHint`. This could result in value types being boxed even though the original IL didn't contain any `box` instruction.
This is an attempt to use better types for stack slot variables created by ILReader. The idea is: there aren't many IL instructions that produce "other" value types, and `InferType()` already handles pretty much all of them, so we can use that to assign types to our stack slots.
It's a bit more tricky if the stack is pushed to on multiple branches that join together before the value is used: here the variable type must be suitable for both assignments. In this case, we go back to the previously-used stacktype.
This also matters in the `1 => DateTime.Now, 2 => null` case -- BestCommonType infers `DateTime` here, but we need `DateTime?` instead. But both had `StackType.O` so this went wrong prior to this commit.
Daniel Grunwald
Siegfried Pammer
sonyps5201314