Fix #3824: fold a hoisted argument null-guard at the ILAst level

ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs

@@ -69,6 +69,31 @@ public Issue1743(int dummy1, int dummy2)
 			}
 		}
 
+		// Multiple constructors that do not chain with this and assign fields differently must not
+		// prevent the remaining constructor's this(...) chain from being lifted to an initializer.
+		public struct StructWithDivergentCtorsAndThisChain
+		{
+			public int X;
+			public int Y;
+
+			public StructWithDivergentCtorsAndThisChain(int x, int y)
+			{
+				X = x;
+				Y = y;
+			}
+
+			public StructWithDivergentCtorsAndThisChain(int x)
+			{
+				X = x;
+				Y = 0;
+			}
+
+			public StructWithDivergentCtorsAndThisChain(string s)
+				: this(s.Length)
+			{
+			}
+		}
+
 #if CS120
 		public struct Issue1743WithPrimaryCtor(int dummy1, int dummy2)
 		{
@@ -232,6 +257,170 @@ public NoRecordButCopyConstructorLike(NoRecordButCopyConstructorLike parent)
 			}
 		}
 
+#if CS70
+		public class NullCheckedArgumentBase
+		{
+			public NullCheckedArgumentBase(int a, int b, int c, string s)
+			{
+			}
+		}
+
+		public class NullCheckedArgumentChain : NullCheckedArgumentBase
+		{
+			public string Value;
+
+			// 'value?.Length ?? throw ...' combined with a later reuse of 'value' makes the compiler
+			// hoist the null-check in front of the chained constructor call. The decompiler must fold
+			// that guard back into the first argument as 'value ?? throw ...' rather than emit an
+			// illegal in-body 'base..ctor(...)' call.
+			public NullCheckedArgumentChain(string value)
+#if EXPECTED_OUTPUT
+				: base(0, (value ?? throw new ArgumentNullException("value")).Length, value.Length, "value")
+#else
+				: base(0, value?.Length ?? throw new ArgumentNullException("value"), value.Length, "value")
+#endif
+			{
+				Value = value;
+			}
+		}
+
+		public class NullCheckedArgumentThisChain
+		{
+			public string Value;
+
+			public NullCheckedArgumentThisChain(int a, int b, int c, string s)
+			{
+			}
+
+			public NullCheckedArgumentThisChain(string value)
+#if EXPECTED_OUTPUT
+				: this(0, (value ?? throw new ArgumentNullException("value")).Length, value.Length, "value")
+#else
+				: this(0, value?.Length ?? throw new ArgumentNullException("value"), value.Length, "value")
+#endif
+			{
+				Value = value;
+			}
+		}
+
+		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)
+#if EXPECTED_OUTPUT
+				: this(0, (value ?? throw new ArgumentNullException("value")).Length, value.Length, "value")
+#else
+				: this(0, value?.Length ?? throw new ArgumentNullException("value"), value.Length, "value")
+#endif
+			{
+				Leet += value.Length;
+			}
+
+			public NullCheckedStructThisChain(int a, int b, int c, string s)
+			{
+				Leet = a + b + c;
+			}
+		}
+
+		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)
+#if EXPECTED_OUTPUT
+				: this((a ?? throw new ArgumentNullException("a")).Length, (b ?? throw new ArgumentNullException("b")).Length, b.Length, a)
+#else
+				: this(a?.Length ?? throw new ArgumentNullException("a"), b?.Length ?? throw new ArgumentNullException("b"), b.Length, a)
+#endif
+			{
+				Leet += a.Length;
+			}
+
+			public NullCheckedStructTwoArguments(int a, int b, int c, string s)
+			{
+				Leet = a + b + c;
+			}
+		}
+
+		public class NullCheckedTwoArguments : NullCheckedArgumentBase
+		{
+			// Two reused parameters produce two stacked hoisted guards before the chained call;
+			// each must be folded into the first argument that uses it.
+			public NullCheckedTwoArguments(string a, string b)
+#if EXPECTED_OUTPUT
+				: base((a ?? throw new ArgumentNullException("a")).Length, (b ?? throw new ArgumentNullException("b")).Length, a.Length, b)
+#else
+				: base(a?.Length ?? throw new ArgumentNullException("a"), b?.Length ?? throw new ArgumentNullException("b"), a.Length, b)
+#endif
+			{
+			}
+		}
+
+		public class NullCheckedNullableArgument : NullCheckedArgumentBase
+		{
+			// A nullable value type argument with 'value ?? throw ...' and a later reuse: here the
+			// compiler emits its own Nullable<T> copy plus a HasValue check in the middle of the
+			// argument evaluation (instead of hoisting a comparison against null), which the
+			// value-type throw-expression fold reassembles.
+			public NullCheckedNullableArgument(int? value)
+				: base(value ?? throw new ArgumentNullException("value"), value.Value, 0, "value")
+			{
+			}
+		}
+
+		public struct NullCheckedNullableStructThisChain
+		{
+			public int Leet;
+
+			public NullCheckedNullableStructThisChain(int? value)
+				: this(value ?? throw new ArgumentNullException("value"), value.Value, 0, "value")
+			{
+				Leet += value.Value;
+			}
+
+			public NullCheckedNullableStructThisChain(int a, int b, int c, string s)
+			{
+				Leet = a + b + c;
+			}
+		}
+
+		public class NullCheckedFirstArgument : NullCheckedArgumentBase
+		{
+			// The guarded parameter is used by the very first argument, so the fold targets argument
+			// index 0.
+			public NullCheckedFirstArgument(string value)
+#if EXPECTED_OUTPUT
+				: base((value ?? throw new ArgumentNullException("value")).Length, value.Length, 0, "value")
+#else
+				: base(value?.Length ?? throw new ArgumentNullException("value"), value.Length, 0, "value")
+#endif
+			{
+			}
+		}
+
+		public class NotHoistedBodyGuard
+		{
+			public string Value;
+
+			// An ordinary argument-validation guard runs after the (implicit) base call, so it must
+			// stay an in-body statement and must not be folded into an initializer.
+			public NotHoistedBodyGuard(string value)
+			{
+				if (value == null)
+				{
+					throw new ArgumentNullException("value");
+				}
+				Value = value;
+			}
+		}
+#endif
+
 #if CS100
 		public class PrimaryCtorClassThisChain(Guid id)
 		{

ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs

@@ -430,13 +430,27 @@ public bool Analyze(IEnumerable<AstNode> members)
 						if (sequence == null)
 							return false;
 
+						bool sequenceMatchesAllCtors = true;
 						for (int i = 1; i < constructorsNotChainedWithThis.Count; i++)
 						{
 							if (!sequence.IsMatch(constructorsNotChainedWithThis[i]))
-								return false;
+							{
+								sequenceMatchesAllCtors = false;
+								break;
+							}
 						}
 
-						if (!isPrimaryCtor)
+						if (!sequenceMatchesAllCtors)
+						{
+							// The non-this-chained constructors disagree on their leading field
+							// assignments, so there is no shared field-initializer sequence to extract.
+							// A primary constructor must extract its initializers (its parameters drive
+							// them), so bail; otherwise keep the assignments in the bodies but continue,
+							// so the this(...)/base(...) chains still get lifted to initializers.
+							if (isPrimaryCtor)
+								return false;
+						}
+						else if (!isPrimaryCtor)
 						{
 							if (!sequence.Statements.Any(s => s.DependsOnConstructorBody))
 								InstanceInitializers = sequence;