pimhwang
/
ILSpy
mirror of https://github.com/icsharpcode/ILSpy.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

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

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
fix-3860-outvar-explicit-type
Siegfried Pammer 2 days ago committed by Siegfried Pammer
parent
51f153d06d
commit
5d49a2f751
2 changed files with 56 additions and 97 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 41
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs
  2. 112
      ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs

41
ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs
Unescape View File

@ -307,18 +307,26 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
{ {
public int Leet; public int Leet;
// Value types chain via 'this = new TSelf(...)'; ChainedConstructorCallILOffset only // Value types chain via 'this = new TSelf(...)', an ordinary body statement, so the
// reports reference-type chained calls, so the hoisted guard fold must locate the struct // hoisted argument null-guard may legally precede it: the guard is deliberately not
// this(...) call itself, otherwise the guard survives and defeats the initializer. // folded back and the chain is not lifted into a this(...) initializer.
public NullCheckedStructThisChain(string value)
#if EXPECTED_OUTPUT #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 #else
public NullCheckedStructThisChain(string value)
: this(0, value?.Length ?? throw new ArgumentNullException("value"), value.Length, "value") : this(0, value?.Length ?? throw new ArgumentNullException("value"), value.Length, "value")
#endif
{ {
Leet += value.Length; Leet += value.Length;
} }
#endif
public NullCheckedStructThisChain(int a, int b, int c, string s) public NullCheckedStructThisChain(int a, int b, int c, string s)
{ {
@ -331,16 +339,29 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
public int Leet; public int Leet;
// Two reused parameters before a value-type this(...) chain produce two stacked hoisted // Two reused parameters before a value-type this(...) chain produce two stacked hoisted
// guards; both must be folded. // guards; both stay in the body (see NullCheckedStructThisChain).
public NullCheckedStructTwoArguments(string a, string b)
#if EXPECTED_OUTPUT #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 #else
public NullCheckedStructTwoArguments(string a, string b)
: this(a?.Length ?? throw new ArgumentNullException("a"), b?.Length ?? throw new ArgumentNullException("b"), b.Length, a) : this(a?.Length ?? throw new ArgumentNullException("a"), b?.Length ?? throw new ArgumentNullException("b"), b.Length, a)
#endif
{ {
Leet += a.Length; Leet += a.Length;
} }
#endif
public NullCheckedStructTwoArguments(int a, int b, int c, string s) public NullCheckedStructTwoArguments(int a, int b, int c, string s)
{ {

112
ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs
Unescape View File

@ -124,11 +124,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
/// redirecting the first following use of the parameter (the position the check was hoisted /// 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 /// 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. /// 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 /// Only reference-type chains (<c>base/this..ctor</c> calls, identified by IL offset via
/// <see cref="ILInlining.IsInConstructorInitializer"/>; value types chain via /// <see cref="ILInlining.IsInConstructorInitializer"/>) need this: value types chain via
/// <c>this = new TSelf(...)</c>, i.e. <c>stobj(ldthis, newobj TSelf(...))</c>, which /// <c>this = new TSelf(...)</c>, which is an ordinary body statement, so a preceding
/// <see cref="ILFunction.ChainedConstructorCallILOffset"/> does not report, so the stobj /// guard statement is legal C# and can stay.
/// shape is matched directly.
/// </summary> /// </summary>
bool TransformHoistedConstructorArgumentNullGuard(Block block, int pos, StatementTransformContext context) bool TransformHoistedConstructorArgumentNullGuard(Block block, int pos, StatementTransformContext context)
{ {
@ -145,26 +144,37 @@ namespace ICSharpCode.Decompiler.IL.Transforms
if (!IsArgumentNullGuard(guard, out var paramLoad, out var throwInst)) if (!IsArgumentNullGuard(guard, out var paramLoad, out var throwInst))
return false; 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; return false;
// Redirect the first following use of the parameter, i.e. the position where the // The compiler hoists the guard directly in front of the statement containing the
// null-check sat before the compiler hoisted it. // parameter use it was lifted out of, so that use must be order-safely reachable
LdLoc firstUse = null; // within the immediately following statement.
for (int i = pos + 1; i <= searchEndPos && firstUse == null; i++) 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>() // reset the primary positions (see remarks on ILInstruction.Parent)
.FirstOrDefault(ld => ld.Variable == paramLoad.Variable); 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); 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); var temp = function.RegisterVariable(VariableKind.StackSlot, paramLoad.Variable.Type);
firstUse.Variable = temp; firstUse.Variable = temp;
throwInst.resultType = StackType.O; throwInst.resultType = StackType.O;
var stloc = new StLoc(temp, new NullCoalescingInstruction(NullCoalescingKind.Ref, paramLoad, throwInst)); var stloc = new StLoc(temp, expressionWithThrow);
stloc.AddILRange(guard); stloc.AddILRange(guard);
block.Instructions[pos] = stloc; block.Instructions[pos] = stloc;
context.EndStep(stloc); context.EndStep(stloc);
@ -194,78 +204,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return true; 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> /// <summary>
/// stloc v(value) /// stloc v(value)
/// if (logic.not(call get_HasValue(ldloca v))) throw(...) /// if (logic.not(call get_HasValue(ldloca v))) throw(...)

Write Preview
Loading…
Cancel
Save