Skip to content

Simplify hoisted null-guard fold to reference-type constructor chains #3863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
siegfriedpammer merged 1 commit into from
Jul 5, 2026
+56 −97
Merged

Simplify hoisted null-guard fold to reference-type constructor chains #3863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,18 +307,26 @@ public struct NullCheckedStructThisChain
{
public int Leet;

// Value types chain via 'this = new TSelf(...)'; ChainedConstructorCallILOffset only
// reports reference-type chained calls, so the hoisted guard fold must locate the struct
// this(...) call itself, otherwise the guard survives and defeats the initializer.
public NullCheckedStructThisChain(string value)
// Value types chain via 'this = new TSelf(...)', an ordinary body statement, so the
// hoisted argument null-guard may legally precede it: the guard is deliberately not
// folded back and the chain is not lifted into a this(...) initializer.
#if EXPECTED_OUTPUT
: this(0, (value ?? throw new ArgumentNullException("value")).Length, value.Length, "value")
public NullCheckedStructThisChain(string value)
{
if (value == null)
{
throw new ArgumentNullException("value");
}
this = new NullCheckedStructThisChain(0, value.Length, value.Length, "value");
Leet += value.Length;
}
#else
public NullCheckedStructThisChain(string value)
: this(0, value?.Length ?? throw new ArgumentNullException("value"), value.Length, "value")
#endif
{
Leet += value.Length;
}
#endif

public NullCheckedStructThisChain(int a, int b, int c, string s)
{
Expand All @@ -331,16 +339,29 @@ public struct NullCheckedStructTwoArguments
public int Leet;

// Two reused parameters before a value-type this(...) chain produce two stacked hoisted
// guards; both must be folded.
public NullCheckedStructTwoArguments(string a, string b)
// guards; both stay in the body (see NullCheckedStructThisChain).
#if EXPECTED_OUTPUT
: this((a ?? throw new ArgumentNullException("a")).Length, (b ?? throw new ArgumentNullException("b")).Length, b.Length, a)
public NullCheckedStructTwoArguments(string a, string b)
{
if (a == null)
{
throw new ArgumentNullException("a");
}
int length = a.Length;
if (b == null)
{
throw new ArgumentNullException("b");
}
this = new NullCheckedStructTwoArguments(length, b.Length, b.Length, a);
Leet += a.Length;
}
#else
public NullCheckedStructTwoArguments(string a, string b)
: this(a?.Length ?? throw new ArgumentNullException("a"), b?.Length ?? throw new ArgumentNullException("b"), b.Length, a)
#endif
{
Leet += a.Length;
}
#endif

public NullCheckedStructTwoArguments(int a, int b, int c, string s)
{
Expand Down
112 changes: 25 additions & 87 deletions ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,10 @@ bool TransformRefTypes(Block block, int pos, StatementTransformContext context)
/// redirecting the first following use of the parameter (the position the check was hoisted
/// from) to <c>temp</c>, and leave moving the coalescing expression into the chained call to
/// ILInlining, which does so only when that preserves the order of evaluation.
/// Reference-type chains (<c>base/this..ctor</c> calls) are identified by IL offset via
/// <see cref="ILInlining.IsInConstructorInitializer"/>; value types chain via
/// <c>this = new TSelf(...)</c>, i.e. <c>stobj(ldthis, newobj TSelf(...))</c>, which
/// <see cref="ILFunction.ChainedConstructorCallILOffset"/> does not report, so the stobj
/// shape is matched directly.
/// Only reference-type chains (<c>base/this..ctor</c> calls, identified by IL offset via
/// <see cref="ILInlining.IsInConstructorInitializer"/>) need this: value types chain via
/// <c>this = new TSelf(...)</c>, which is an ordinary body statement, so a preceding
/// guard statement is legal C# and can stay.
/// </summary>
bool TransformHoistedConstructorArgumentNullGuard(Block block, int pos, StatementTransformContext context)
{
Expand All @@ -145,26 +144,37 @@ bool TransformHoistedConstructorArgumentNullGuard(Block block, int pos, Statemen
if (!IsArgumentNullGuard(guard, out var paramLoad, out var throwInst))
return false;

if (!GuardPrecedesChainedConstructorCall(block, pos, function, out int searchEndPos))
// Only a guard sitting in the initializer's argument evaluation, i.e. before the
// chained constructor call, is necessarily compiler-hoisted.
if (!ILInlining.IsInConstructorInitializer(function, guard))
return false;

// Redirect the first following use of the parameter, i.e. the position where the
// null-check sat before the compiler hoisted it.
LdLoc firstUse = null;
for (int i = pos + 1; i <= searchEndPos && firstUse == null; i++)
// The compiler hoists the guard directly in front of the statement containing the
// parameter use it was lifted out of, so that use must be order-safely reachable
// within the immediately following statement.
var paramLoadParent = paramLoad.Parent;
var paramLoadChildIndex = paramLoad.ChildIndex;
var throwInstParent = throwInst.Parent;
var throwInstChildIndex = throwInst.ChildIndex;
var expressionWithThrow = new NullCoalescingInstruction(NullCoalescingKind.Ref, paramLoad, throwInst);
var result = ILInlining.FindLoadInNext(block.Instructions[pos + 1], paramLoad.Variable, expressionWithThrow,
InliningOptions.None);
if (result.Type != ILInlining.FindResultType.Found || result.LoadInst is not LdLoc firstUse)
{
firstUse = block.Instructions[i].Descendants.OfType<LdLoc>()
.FirstOrDefault(ld => ld.Variable == paramLoad.Variable);
// reset the primary positions (see remarks on ILInstruction.Parent)

@dgrunwald dgrunwald Jul 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A simpler alternative might be to use the original IfInstruction as inlinedExpression for the FindLoadInNext call; then construct the new expression only if the transform is possible.

var paramLoadSiblings = paramLoadParent.Children;
paramLoadSiblings[paramLoadChildIndex] = paramLoad;
var throwInstSiblings = throwInstParent.Children;
throwInstSiblings[throwInstChildIndex] = throwInst;
return false;
}
if (firstUse == null)
return false; // parameter not used up to the chained call -> cannot fold; leave guard in place

context.Step($"NullCoalescingTransform: fold hoisted null-guard of '{paramLoad.Variable.Name}' into argument of chained constructor call", guard);

var temp = function.RegisterVariable(VariableKind.StackSlot, paramLoad.Variable.Type);
firstUse.Variable = temp;
throwInst.resultType = StackType.O;
var stloc = new StLoc(temp, new NullCoalescingInstruction(NullCoalescingKind.Ref, paramLoad, throwInst));
var stloc = new StLoc(temp, expressionWithThrow);
stloc.AddILRange(guard);
block.Instructions[pos] = stloc;
context.EndStep(stloc);
Expand Down Expand Up @@ -194,78 +204,6 @@ static bool IsArgumentNullGuard(ILInstruction inst, out LdLoc paramLoad, out Thr
return true;
}

/// <summary>
/// Determines whether the guard at <paramref name="pos"/> precedes the constructor's chained
/// this/base call, i.e. belongs to the hoisted argument evaluation of the constructor
/// initializer. <paramref name="searchEndPos"/> is the last statement index that may contain
/// the parameter use to redirect (the statement containing the chained call, if it is in
/// this block).
/// </summary>
static bool GuardPrecedesChainedConstructorCall(Block block, int pos, ILFunction function, out int searchEndPos)
{
searchEndPos = -1;
if (ILInlining.IsInConstructorInitializer(function, block.Instructions[pos]))
{
// Reference-type chain: everything before ChainedConstructorCallILOffset is the
// initializer's argument evaluation. Search up to and including the first statement
// that reaches past that offset (the statement containing the chained call).
int ctorCallStart = function.ChainedConstructorCallILOffset;
for (int i = pos + 1; i < block.Instructions.Count; i++)
{
searchEndPos = i;
if (block.Instructions[i].EndILOffset > ctorCallStart)
break;
}
return searchEndPos > pos;
}
// Value-type chain: `this = new TSelf(...)` is not reported by
// ChainedConstructorCallILOffset, so match the stobj shape directly. Only further
// hoisted guards may sit between this guard and the chained call; anything else means
// the stobj is a plain body statement rather than a chain.
for (int i = pos + 1; i < block.Instructions.Count; i++)
{
var inst = block.Instructions[i];
if (IsValueTypeChainedConstructorCall(inst, function))
{
searchEndPos = i;
return true;
}
if (!IsArgumentNullGuard(inst, out _, out _))
return false;
}
return false;
}

/// <summary>
/// True if <paramref name="inst"/> is a value-type chained constructor call
/// <c>this = new TSelf(...)</c>, i.e. <c>stobj(ldthis, newobj TSelf(...))</c> where TSelf
/// is the constructor's declaring type.
/// </summary>
static bool IsValueTypeChainedConstructorCall(ILInstruction inst, ILFunction function)
{
return inst is StObj { Value: NewObj { Method.IsConstructor: true } newObj } stobj
&& newObj.Method.DeclaringType.IsReferenceType == false
&& newObj.Method.DeclaringTypeDefinition == function.Method.DeclaringTypeDefinition
&& MatchLdThisOrStackSlotCopy(stobj.Target);
}

/// <summary>
/// Matches a load of the this-pointer, either directly or via a single-definition stack slot
/// that copies it. The compiler spills this to such a slot when a hoisted guard sits between
/// the this-load and the chained <c>this = new TSelf(...)</c> call.
/// </summary>
static bool MatchLdThisOrStackSlotCopy(ILInstruction inst)
{
if (inst.MatchLdThis())
return true;
return inst.MatchLdLoc(out var v)
&& v.Kind == VariableKind.StackSlot
&& v.IsSingleDefinition
&& v.StoreInstructions.Count == 1
&& v.StoreInstructions[0] is StLoc { Value: { } storeValue }
&& storeValue.MatchLdThis();
}

/// <summary>
/// stloc v(value)
/// if (logic.not(call get_HasValue(ldloca v))) throw(...)
Expand Down
Loading