Browse Source

Merge pull request #4064 from icsharpcode/exp/ternary-chain-limit

Expand nested conditional operators back into if-else
pull/4082/head
Siegfried Pammer 2 weeks ago committed by GitHub
parent
commit
51d3d3c6e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 6
      ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs
  2. 6
      ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs
  3. 27
      ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ConditionalChain.cs
  4. 119
      ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ConditionalChain.il
  5. 144
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConditionalOperatorNesting.cs
  6. 1
      ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
  7. 39
      ICSharpCode.Decompiler/IL/ILTypeExtensions.cs
  8. 222
      ICSharpCode.Decompiler/IL/Transforms/ExpandNestedConditionals.cs

6
ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs

@ -51,6 +51,12 @@ namespace ICSharpCode.Decompiler.Tests @@ -51,6 +51,12 @@ namespace ICSharpCode.Decompiler.Tests
}
}
[Test]
public async Task ConditionalChain()
{
await Run();
}
[Test]
public async Task AnonymousMethodEdgeCases()
{

6
ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs

@ -457,6 +457,12 @@ namespace ICSharpCode.Decompiler.Tests @@ -457,6 +457,12 @@ namespace ICSharpCode.Decompiler.Tests
await RunForLibrary(cscOptions: cscOptions);
}
[Test]
public async Task ConditionalOperatorNesting([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions);
}
[Test]
public async Task UnsafeCode([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
{

27
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ConditionalChain.cs

@ -0,0 +1,27 @@ @@ -0,0 +1,27 @@
using System;
public class ConditionalChain
{
public int Ladder(int n, int[] a, int[] b)
{
int num;
if (n > 40)
{
num = a[0];
}
else if (n > 30)
{
num = b[0];
}
else if (n > 20)
{
num = a[1];
}
else
{
num = ((n > 10) ? b[1] : a[2]);
}
Console.WriteLine(num);
return num;
}
}

119
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/ConditionalChain.il

@ -0,0 +1,119 @@ @@ -0,0 +1,119 @@
// .NET IL Disassembler. Version 10.0.11
// Metadata version: v4.0.30319
.assembly extern System.Runtime
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
.ver 4:0:0:0
}
.assembly extern System.Console
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
.ver 4:0:0:0
}
.assembly ConditionalChain
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 )
.custom instance void [System.Runtime]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78 // ....T..WrapNonEx
63 65 70 74 69 6F 6E 54 68 72 6F 77 73 01 ) // ceptionThrows.
// --- The following custom attribute is added automatically, do not uncomment -------
// .custom instance void [System.Runtime]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.DebuggableAttribute/DebuggingModes) = ( 01 00 07 01 00 00 00 00 )
}
.module ConditionalChain.dll
// MVID: {db181304-d3cd-4187-9018-312550b7e28d}
.custom instance void [System.Runtime]System.Runtime.CompilerServices.RefSafetyRulesAttribute::.ctor(int32) = ( 01 00 0B 00 00 00 00 00 )
.imagebase 0x10000000
.file alignment 0x00000200
.stackreserve 0x00100000
.subsystem 0x0003 // WINDOWS_CUI
.corflags 0x00000001 // ILONLY
// Image base: 0x0x7fc5d693d000
// =============== CLASS MEMBERS DECLARATION ===================
.class public auto ansi beforefieldinit ConditionalChain
extends [System.Runtime]System.Object
{
.method public hidebysig instance int32
Ladder(int32 n,
int32[] a,
int32[] b) cil managed
{
// Code size 58 (0x3a)
.maxstack 2
.locals init (int32 V_0,
int32 V_1)
IL_0000: nop
IL_0001: ldarg.1
IL_0002: ldc.i4.s 40
IL_0004: bgt.s IL_0029
IL_0006: ldarg.1
IL_0007: ldc.i4.s 30
IL_0009: bgt.s IL_0024
IL_000b: ldarg.1
IL_000c: ldc.i4.s 20
IL_000e: bgt.s IL_001f
IL_0010: ldarg.1
IL_0011: ldc.i4.s 10
IL_0013: bgt.s IL_001a
IL_0015: ldarg.2
IL_0016: ldc.i4.2
IL_0017: ldelem.i4
IL_0018: br.s IL_001d
IL_001a: ldarg.3
IL_001b: ldc.i4.1
IL_001c: ldelem.i4
IL_001d: br.s IL_0022
IL_001f: ldarg.2
IL_0020: ldc.i4.1
IL_0021: ldelem.i4
IL_0022: br.s IL_0027
IL_0024: ldarg.3
IL_0025: ldc.i4.0
IL_0026: ldelem.i4
IL_0027: br.s IL_002c
IL_0029: ldarg.2
IL_002a: ldc.i4.0
IL_002b: ldelem.i4
IL_002c: stloc.0
IL_002d: ldloc.0
IL_002e: call void [System.Console]System.Console::WriteLine(int32)
IL_0033: nop
IL_0034: ldloc.0
IL_0035: stloc.1
IL_0036: br.s IL_0038
IL_0038: ldloc.1
IL_0039: ret
} // end of method ConditionalChain::Ladder
.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 8 (0x8)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
IL_0006: nop
IL_0007: ret
} // end of method ConditionalChain::.ctor
} // end of class ConditionalChain
// =============================================================
// *********** DISASSEMBLY COMPLETE ***********************

144
ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConditionalOperatorNesting.cs

@ -0,0 +1,144 @@ @@ -0,0 +1,144 @@
// Copyright (c) 2026 Siegfried Pammer
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System;
using System.Collections.Generic;
using System.Linq;
namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
{
public class ConditionalOperatorNesting
{
private static void Use(object value)
{
}
public int ShallowChainStaysInline(int n, int[] a, int[] b)
{
return ((n > 20) ? a[0] : b[0]) + 1;
}
// One conditional is under the limit and stays where it is, whatever the branch types.
public void MismatchedBranchTypesUnderTheLimitStayInline(int n, string s, object o)
{
Use((n > 10) ? s : o);
}
public void LogicOperatorsAreNotCounted(int n, int[] a)
{
Use(n > 40 && a[0] > 1 && a[1] > 2 && a[2] > 3 && a[3] > 4);
}
public int FilterKeepsExpression(int n, int[] a, int[] b)
{
try
{
throw new Exception();
}
catch (Exception) when (((n > 40) ? a[0] : ((n > 30) ? b[0] : ((n > 20) ? a[1] : ((n > 10) ? b[1] : a[2])))) > 5)
{
return 1;
}
}
}
public class ConditionalOperatorNestingBase
{
public ConditionalOperatorNestingBase(int x)
{
}
}
public class ConditionalOperatorNestingCtor : ConditionalOperatorNestingBase
{
public ConditionalOperatorNestingCtor(int n, int[] a, int[] b)
: base((n > 40) ? a[0] : ((n > 30) ? b[0] : ((n > 20) ? a[1] : ((n > 10) ? b[1] : a[2]))))
{
}
public ConditionalOperatorNestingCtor(int n, int[] a)
: this(n, a, a)
{
}
}
#if CS90
public class ConditionalOperatorNestingInit
{
public int P { get; init; }
}
public record ConditionalOperatorNestingRecord
{
public int P { get; init; }
}
#endif
// A nested conditional feeding a construct that has to stay a single expression must not be
// expanded: the transform that recognizes the construct matches on the expression, and an
// if-else between the statements silently stops it matching - or produces code that does not
// compile, as an object initializer assigning an init-only member would.
public class ConditionalOperatorNestingSingleExpression
{
public int[] ArrayInitializer(int n, int a, int b, int c)
{
return new int[2] {
(n > 2) ? a : ((n > 1) ? b : c),
5
};
}
public IEnumerable<int> Query(int[] xs, int n, int a, int b, int c)
{
return xs.Where((int x) => x > ((n > 2) ? a : ((n > 1) ? b : c)));
}
#if CS70
public int RefLocal(int n, int[] a, int[] b)
{
ref int reference = ref n > 2 ? ref a[0] : ref n > 1 ? ref b[0] : ref a[1];
reference++;
return reference;
}
#endif
#if CS80
public int SwitchExpression(int k, int n, int a, int b, int c)
{
return k switch {
1 => (n > 2) ? a : ((n > 1) ? b : c),
2 => a,
_ => b,
};
}
#endif
#if CS90
public ConditionalOperatorNestingInit ObjectInitializer(int n, int a, int b, int c)
{
return new ConditionalOperatorNestingInit {
P = ((n > 2) ? a : ((n > 1) ? b : c))
};
}
public ConditionalOperatorNestingRecord WithExpression(ConditionalOperatorNestingRecord i, int n, int a, int b, int c)
{
return i with {
P = ((n > 2) ? a : ((n > 1) ? b : c))
};
}
#endif
}
}

1
ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@ -174,6 +174,7 @@ namespace ICSharpCode.Decompiler.CSharp @@ -174,6 +174,7 @@ namespace ICSharpCode.Decompiler.CSharp
new IntroduceDynamicTypeOnLocals(),
new IntroduceNativeIntTypeOnLocals(),
new IntroduceScopedModifierOnLocals(),
new ExpandNestedConditionals(),
new AssignVariableNames(),
};
}

39
ICSharpCode.Decompiler/IL/ILTypeExtensions.cs

@ -163,6 +163,45 @@ namespace ICSharpCode.Decompiler.IL @@ -163,6 +163,45 @@ namespace ICSharpCode.Decompiler.IL
}
}
/// <summary>
/// Infers the C# type an instruction expects of the child in <paramref name="childIndex"/>,
/// i.e. the counterpart to <see cref="InferType"/>: that one asks what a value is, this one
/// asks what the position it flows into says it should be.
///
/// Returns SpecialType.UnknownType where the position names nothing.
/// </summary>
/// <remarks>
/// Where a value's own type is only its stack type - `I4` being `int`, `bool`, `char` and
/// every enum at once - the consumer often still knows, because a parameter, a return type
/// or a field carries its type in metadata.
/// </remarks>
public static IType InferExpectedType(this ILInstruction inst, int childIndex, ICompilation? compilation)
{
switch (inst)
{
case CallInstruction call:
if (childIndex == 0 && call.IsInstanceCall)
return call.ConstrainedTo ?? call.Method.DeclaringType;
return call.GetParameter(childIndex)?.Type ?? SpecialType.UnknownType;
case Leave leave when childIndex == 0:
// the value of a leave is a return value only where it leaves the function body
var function = leave.Ancestors.OfType<ILFunction>().FirstOrDefault();
if (function == null || leave.TargetContainer != function.Body)
return SpecialType.UnknownType;
return function.Method?.ReturnType ?? SpecialType.UnknownType;
case StObj stobj when childIndex == 1:
return stobj.Type;
case StLoc stloc when childIndex == 0:
return stloc.Variable.Type;
case IfInstruction ifInst when childIndex == 0:
return compilation?.FindType(KnownTypeCode.Boolean) ?? SpecialType.UnknownType;
case NewArr newArr:
return compilation?.FindType(KnownTypeCode.Int32) ?? SpecialType.UnknownType;
default:
return SpecialType.UnknownType;
}
}
/// <summary>
/// Infers the C# type for an IL instruction.
///

222
ICSharpCode.Decompiler/IL/Transforms/ExpandNestedConditionals.cs

@ -0,0 +1,222 @@ @@ -0,0 +1,222 @@
// Copyright (c) 2026 Siegfried Pammer
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using System.Collections.Generic;
using System.Linq;
using ICSharpCode.Decompiler.TypeSystem;
namespace ICSharpCode.Decompiler.IL.Transforms
{
/// <summary>
/// Expands a statement-level assignment of a nested conditional operator back into if-else.
///
/// `ExpressionTransforms.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
/// issue #2027 decompiles to a single 2095-character statement.
///
/// This undoes the collapse past <see cref="MaxNesting"/> levels, so a statement keeps at most
/// that many conditional operators.
/// </summary>
/// <remarks>
/// This runs at the end of the pipeline, not 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.
///
/// Only a store to a declared local is expanded. Such a local carries its own type from metadata,
/// so the branches can be split without the result widening to the stack type - which is what
/// makes a separate type check unnecessary here.
/// </remarks>
public class ExpandNestedConditionals : IILTransform
{
/// <summary>
/// How many conditional operators a single statement may keep.
/// </summary>
const int MaxNesting = 1;
ILTransformContext context;
public void Run(ILFunction function, ILTransformContext context)
{
this.context = context;
if (context.Settings.AggressiveInlining)
return;
foreach (var f in function.Descendants.OfType<ILFunction>())
{
if (!IsExpandableFunction(f))
continue;
foreach (var block in f.Descendants.OfType<Block>().ToArray())
{
if (block.Ancestors.OfType<ILFunction>().FirstOrDefault() != f)
continue;
if (!IsExpandableBlock(block))
continue;
for (int i = 0; i < block.Instructions.Count; i++)
{
context.CancellationToken.ThrowIfCancellationRequested();
ExtractInto(block, i);
Expand(f, block, i);
}
}
}
}
/// <summary>
/// A lambda body may have to stay a single expression: the query-expression stage rewrites
/// one into a clause, and an expression tree is built from the expression itself. A
/// constructor is matched as a whole, so that its leading stores become field initializers
/// and a record's primary constructor is recognized.
/// </summary>
static bool IsExpandableFunction(ILFunction function)
{
return function.Kind is ILFunctionKind.TopLevelFunction or ILFunctionKind.LocalFunction
&& function.Method?.IsConstructor != true;
}
/// <summary>
/// Only plain control-flow blocks hold statements. The other block kinds are expressions
/// spelled as blocks - an initializer, a named-argument call - and an if-else inside one of
/// them is not a statement the C# stage can print.
/// </summary>
static bool IsExpandableBlock(Block block)
{
if (block.Kind != BlockKind.ControlFlow)
return false;
if (ILInlining.IsCatchWhenBlock(block))
return false;
// A branch body is a block hanging off the if, not off a container; it holds statements
// either way. Only a container's own layout reserves blocks for a loop or switch header.
if (block.Parent is not BlockContainer container)
return true;
return container.Kind switch {
// the entry point carries the loop condition or the switch value
ContainerKind.While or ContainerKind.Switch => block != container.EntryPoint,
// and for a for-loop the last block carries the increment
ContainerKind.For => block != container.EntryPoint
&& block != container.Blocks[container.Blocks.Count - 1],
ContainerKind.DoWhile => block != container.Blocks[container.Blocks.Count - 1],
_ => true,
};
}
/// <summary>
/// A conditional that is not stored to a variable - an argument, a return value, a
/// condition - has nothing to expand into. ILExtraction gives it one, vetting the move
/// through PrepareExtract so the order of evaluation is preserved.
/// </summary>
/// <remarks>
/// Extraction is only done where the position the value flows into names a type. The
/// temporary ILExtraction creates is typed from the stack type, and `I4` is `int`, `bool`,
/// `char` and every enum at once; the consumer knows better, because a parameter, a return
/// type or a field carries its type in metadata.
/// </remarks>
void ExtractInto(Block block, int pos)
{
if (block.Instructions[pos] is StLoc { Value: IfInstruction })
return;
foreach (var inst in block.Instructions[pos].Descendants.OfType<IfInstruction>().ToArray())
{
if (inst.Parent is StLoc || !ExceedsNesting(inst, MaxNesting))
continue;
var expected = inst.Parent?.InferExpectedType(inst.ChildIndex, context.TypeSystem);
if (expected == null || expected.Kind == TypeKind.Unknown
|| expected.GetStackType() != inst.ResultType)
{
continue;
}
context.Step("Extract nested conditional operator", inst);
var v = inst.Extract(context);
if (v != null)
{
v.Type = expected;
context.EndStep(block.Instructions[pos]);
return;
}
context.EndStep(inst);
}
}
void Expand(ILFunction function, Block block, int pos)
{
// Only one branch of a chain nests further, so the expansion walks down it. A chain is
// bounded only by the size of the method it came from, hence the loop over recursion.
var worklist = new Stack<(Block Block, int Pos)>();
worklist.Push((block, pos));
while (worklist.Count > 0)
{
var (current, index) = worklist.Pop();
if (current.Instructions[index] is not StLoc { Variable: var v, Value: IfInstruction ifInst } stloc)
continue;
if (v.Kind is not (VariableKind.Local or VariableKind.StackSlot) || v.Type is ByReferenceType)
continue;
if (ILInlining.IsInConstructorInitializer(function, stloc))
continue;
if (!ExceedsNesting(ifInst, MaxNesting))
continue;
context.Step("Expand nested conditional operator", stloc);
// HandleConditionalOperator built the conditional by negating the condition and
// swapping the branches; undoing that here keeps the source's own polarity, and
// makes the expansion the exact inverse of the collapse rather than an equivalent
// of it - without which a pretty-test fixture would never reach a fixed point.
var condition = ifInst.Condition;
var (trueValue, falseValue) = (ifInst.TrueInst, ifInst.FalseInst);
while (condition.MatchLogicNot(out var withoutNot))
{
condition = withoutNot;
(trueValue, falseValue) = (falseValue, trueValue);
}
// Each new store stands where its value did, so the statement keeps a sequence point
// of its own instead of inheriting the one the value already carries.
var trueStore = new StLoc(v, trueValue).WithILRange(trueValue);
var falseStore = new StLoc(v, falseValue).WithILRange(falseValue);
var trueBlock = new Block { Instructions = { trueStore } }.WithILRange(trueStore);
var falseBlock = new Block { Instructions = { falseStore } }.WithILRange(falseStore);
var expanded = new IfInstruction(condition, trueBlock, falseBlock);
expanded.AddILRange(ifInst);
expanded.AddILRange(stloc);
stloc.ReplaceWith(expanded);
context.EndStep(expanded);
// the branches are statements of their own now, so each may need expanding in turn
worklist.Push((trueBlock, 0));
worklist.Push((falseBlock, 0));
}
}
/// <summary>
/// Gets whether the branches of <paramref name="inst"/> nest conditional operators
/// <paramref name="depth"/> levels deep. The short-circuit logic operators share the
/// IfInstruction representation, but render as operator chains without visible nesting.
/// </summary>
static bool ExceedsNesting(ILInstruction inst, int depth)
{
if (inst is not IfInstruction ifInst)
return false;
if (ifInst.MatchLogicAnd(out _, out _) || ifInst.MatchLogicOr(out _, out _))
return false;
return depth <= 0
|| ExceedsNesting(ifInst.TrueInst, depth - 1)
|| ExceedsNesting(ifInst.FalseInst, depth - 1);
}
}
}
Loading…
Cancel
Save