Merge pull request #3712 from nikitalita/fix-SequencePointBuilder

Fix SequencePointBuilder skipping moved member initializers

ICSharpCode.Decompiler.Tests/PdbGenerationTestRunner.cs

@@ -62,6 +62,45 @@ namespace ICSharpCode.Decompiler.Tests
 			TestSequencePoints(knownResidual: true);
 		}
 
+		[Test]
+		public void MemberInitializers()
+		{
+			(string peFileName, _) = CompileTestCase(nameof(MemberInitializers));
+
+			var module = new PEFile(peFileName);
+			var resolver = new UniversalAssemblyResolver(peFileName, false,
+				module.Metadata.DetectTargetFrameworkId(), null, PEStreamOptions.PrefetchEntireImage);
+			var decompiler = new CSharpDecompiler(module, resolver, new DecompilerSettings());
+
+			using var generatedPdb = new MemoryStream();
+			new PortablePdbWriter { NoLogo = true }
+				.WritePdb(module, decompiler, new DecompilerSettings(), generatedPdb);
+
+			var methodNames = PdbSequencePoints.ReadMethodNames(peFileName);
+			var actual = PdbSequencePoints.Read(generatedPdb);
+			var constructorRows = methodNames
+				.Where(pair => pair.Value == "MemberInitializers..ctor")
+				.Select(pair => pair.Key)
+				.ToArray();
+
+			Assert.That(constructorRows, Has.Length.EqualTo(2),
+				"the fixture must keep both overloaded constructors");
+			foreach (int row in constructorRows)
+			{
+				Assert.That(actual, Does.ContainKey(row),
+					$"constructor method row 0x{0x06000000 | row:x8} has no generated sequence points");
+				var visibleStartLines = actual[row]
+					.Where(point => !point.IsHidden)
+					.Select(point => point.StartLine)
+					.ToArray();
+
+				Assert.That(visibleStartLines, Does.Contain(5),
+					"the instance field initializer must be mapped in every constructor");
+				Assert.That(visibleStartLines, Does.Contain(7),
+					"the auto-property initializer must be mapped in every constructor");
+			}
+		}
+
 		[Test]
 		public void TryCatchFinally()
 		{
@@ -483,6 +522,51 @@ namespace ICSharpCode.Decompiler.Tests
 			}
 		}
 
+		[Test]
+		public void MemberInitializerSingleCtor()
+		{
+			// The lifted field and auto-property initializers must be breakpointable in the single
+			// explicit constructor.
+			TestSequencePoints(knownResidual: true);
+		}
+
+		[Test]
+		public void MemberInitializerChainedCtors()
+		{
+			// The non-chained constructor breakpoints the initializers; the this()-chained constructor
+			// does not run them and must not.
+			TestSequencePoints(knownResidual: true);
+		}
+
+		[Test]
+		public void MemberInitializerImplicitCtor()
+		{
+			// The compiler-generated default constructor runs the initializers and must breakpoint them.
+			TestSequencePoints(knownResidual: true);
+		}
+
+		[Test]
+		public void MemberInitializerStaticCtor()
+		{
+			// Static initializers run in the implicit static constructor and must be breakpointable there.
+			TestSequencePoints(knownResidual: true);
+		}
+
+		[Test]
+		public void MemberInitializerPrimaryCtor()
+		{
+			// The field initializer runs in the primary constructor (whose body is never printed) and
+			// must be breakpointable there.
+			TestSequencePoints(knownResidual: true);
+		}
+
+		[Test]
+		public void MemberInitializerEvents()
+		{
+			// A field-like event initializer runs in every instance constructor, like a field.
+			TestSequencePoints(knownResidual: true);
+		}
+
 		private static void CompileCSharpWithPdb(string outputBase, string sourceFile)
 		{
 			Tester.CompileCSharpWithPdb(outputBase, new Dictionary<string, string> {

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/LockStatement.residual.txt

@@ -3,4 +3,6 @@
   - compiler only: hidden after (12,3)-(12,4)
   - compiler only: (13,2)-(13,3)
 0x06000003 LockStatement..cctor
+  + decompiler only: (5,40)-(5,52)
+  + decompiler only: hidden after (5,40)-(5,52)
   - compiler only: (5,2)-(5,53)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerChainedCtors.cs

@@ -0,0 +1,30 @@
+using System;
+
+internal class MemberInitializerChainedCtors
+{
+	private int field = Compute(1);
+
+	private int Property { get; } = Compute(2);
+
+	public MemberInitializerChainedCtors()
+	{
+		Console.WriteLine(field + Property);
+	}
+
+	public MemberInitializerChainedCtors(int value)
+		: this()
+	{
+		Console.WriteLine(value);
+	}
+
+	private static int Compute(int value)
+	{
+		return value;
+	}
+
+	private static void Main()
+	{
+		new MemberInitializerChainedCtors();
+		new MemberInitializerChainedCtors(3);
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerChainedCtors.residual.txt

@@ -0,0 +1,11 @@
+0x06000001 MemberInitializerChainedCtors.get_Property
+  - compiler only: (7,25)-(7,29)
+0x06000002 MemberInitializerChainedCtors..ctor
+  + decompiler only: (10,2)-(10,3)
+  + decompiler only: (5,22)-(5,32)
+  - compiler only: (5,2)-(5,33)
+  + decompiler only: hidden after (7,34)-(7,44)
+  - compiler only: (9,2)-(9,40)
+0x06000003 MemberInitializerChainedCtors..ctor
+  + decompiler only: (16,2)-(16,3)
+  - compiler only: (15,5)-(15,11)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerEvents.cs

@@ -0,0 +1,26 @@
+using System;
+
+internal class MemberInitializerEvents
+{
+	private event EventHandler Changed = Handler;
+
+	public MemberInitializerEvents()
+	{
+		this.Changed?.Invoke(this, EventArgs.Empty);
+	}
+
+	public MemberInitializerEvents(int value)
+	{
+		this.Changed?.Invoke(this, EventArgs.Empty);
+	}
+
+	private static void Handler(object sender, EventArgs e)
+	{
+	}
+
+	private static void Main()
+	{
+		new MemberInitializerEvents();
+		new MemberInitializerEvents(3);
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerEvents.residual.txt

@@ -0,0 +1,14 @@
+0x06000003 MemberInitializerEvents..ctor
+  + decompiler only: (8,2)-(8,3)
+  + decompiler only: (5,39)-(5,46)
+  + decompiler only: hidden after (5,39)-(5,46)
+  - compiler only: (5,2)-(5,47)
+  - compiler only: (7,2)-(7,34)
+  + decompiler only: hidden after (10,2)-(10,3)
+0x06000004 MemberInitializerEvents..ctor
+  + decompiler only: (13,2)-(13,3)
+  + decompiler only: (5,39)-(5,46)
+  + decompiler only: hidden after (5,39)-(5,46)
+  - compiler only: (5,2)-(5,47)
+  - compiler only: (12,2)-(12,43)
+  + decompiler only: hidden after (15,2)-(15,3)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerImplicitCtor.cs

@@ -0,0 +1,19 @@
+using System;
+
+internal class MemberInitializerImplicitCtor
+{
+	private int field = Compute(1);
+
+	private int Property { get; } = Compute(2);
+
+	private static int Compute(int value)
+	{
+		return value;
+	}
+
+	private static void Main()
+	{
+		MemberInitializerImplicitCtor memberInitializerImplicitCtor = new MemberInitializerImplicitCtor();
+		Console.WriteLine(memberInitializerImplicitCtor.field + memberInitializerImplicitCtor.Property);
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerImplicitCtor.residual.txt

@@ -0,0 +1,6 @@
+0x06000001 MemberInitializerImplicitCtor.get_Property
+  - compiler only: (7,25)-(7,29)
+0x06000004 MemberInitializerImplicitCtor..ctor
+  + decompiler only: (5,22)-(5,32)
+  - compiler only: (5,2)-(5,33)
+  + decompiler only: hidden after (7,34)-(7,44)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerPrimaryCtor.cs

@@ -0,0 +1,21 @@
+using System;
+
+internal class MemberInitializerPrimaryCtor(int factor)
+{
+	private int field = Compute(factor);
+
+	public override string ToString()
+	{
+		return field.ToString();
+	}
+
+	private static int Compute(int value)
+	{
+		return value;
+	}
+
+	private static void Main()
+	{
+		Console.WriteLine(new MemberInitializerPrimaryCtor(3));
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerPrimaryCtor.residual.txt

@@ -0,0 +1,5 @@
+0x06000001 MemberInitializerPrimaryCtor..ctor
+  + decompiler only: (5,22)-(5,37)
+  + decompiler only: hidden after (5,22)-(5,37)
+  - compiler only: (5,2)-(5,38)
+  - compiler only: (3,16)-(3,56)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerSingleCtor.cs

@@ -0,0 +1,23 @@
+using System;
+
+internal class MemberInitializerSingleCtor
+{
+	private int field = Compute(1);
+
+	private int Property { get; } = Compute(2);
+
+	public MemberInitializerSingleCtor()
+	{
+		Console.WriteLine(field + Property);
+	}
+
+	private static int Compute(int value)
+	{
+		return value;
+	}
+
+	private static void Main()
+	{
+		new MemberInitializerSingleCtor();
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerSingleCtor.residual.txt

@@ -0,0 +1,8 @@
+0x06000001 MemberInitializerSingleCtor.get_Property
+  - compiler only: (7,25)-(7,29)
+0x06000002 MemberInitializerSingleCtor..ctor
+  + decompiler only: (10,2)-(10,3)
+  + decompiler only: (5,22)-(5,32)
+  - compiler only: (5,2)-(5,33)
+  + decompiler only: hidden after (7,34)-(7,44)
+  - compiler only: (9,2)-(9,38)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerStaticCtor.cs

@@ -0,0 +1,18 @@
+using System;
+
+internal class MemberInitializerStaticCtor
+{
+	private static int field = Compute(1);
+
+	private static int Property { get; } = Compute(2);
+
+	private static int Compute(int value)
+	{
+		return value;
+	}
+
+	private static void Main()
+	{
+		Console.WriteLine(field + Property);
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializerStaticCtor.residual.txt

@@ -0,0 +1,6 @@
+0x06000001 MemberInitializerStaticCtor.get_Property
+  - compiler only: (7,32)-(7,36)
+0x06000005 MemberInitializerStaticCtor..cctor
+  + decompiler only: (5,29)-(5,39)
+  - compiler only: (5,2)-(5,40)
+  + decompiler only: hidden after (7,41)-(7,51)

ICSharpCode.Decompiler.Tests/TestCases/PdbGen/MemberInitializers.cs

@@ -0,0 +1,29 @@
+using System;
+
+internal class MemberInitializers
+{
+	private int instanceField = Compute(1);
+
+	private int InstanceProperty { get; } = Compute(2);
+
+	public MemberInitializers()
+	{
+		Console.WriteLine(instanceField + InstanceProperty);
+	}
+
+	public MemberInitializers(int value)
+	{
+		Console.WriteLine(instanceField + InstanceProperty + value);
+	}
+
+	private static int Compute(int value)
+	{
+		return value;
+	}
+
+	private static void Main()
+	{
+		_ = new MemberInitializers();
+		_ = new MemberInitializers(3);
+	}
+}

ICSharpCode.Decompiler/CSharp/Annotations.cs

@@ -286,6 +286,27 @@ namespace ICSharpCode.Decompiler.CSharp
 		}
 	}
 
+	/// <summary>
+	/// Annotates a field/auto-property/event initializer that the decompiler lifted to the
+	/// declaration site with the copies of the same initializer found in the other constructors.
+	/// In IL a member initializer runs in every instance constructor that does not chain to
+	/// this(...), so the initializer's expression appears once per such constructor. The decompiler
+	/// lifts it from a single constructor and discards the rest; this annotation preserves the
+	/// discarded copies so <see cref="SequencePointBuilder"/> can map the initializer's source
+	/// location onto every constructor that runs it, not just the one it was lifted from. Each entry
+	/// is one other constructor's copy of the initializer expression (still carrying that
+	/// constructor's IL annotations).
+	/// </summary>
+	public class MemberInitializerInOtherConstructorsAnnotation
+	{
+		public readonly IReadOnlyList<Expression> Initializers;
+
+		public MemberInitializerInOtherConstructorsAnnotation(IReadOnlyList<Expression> initializers)
+		{
+			this.Initializers = initializers;
+		}
+	}
+
 	/// <summary>
 	/// Annotates an expression when an implicit user-defined conversion was omitted.
 	/// </summary>

ICSharpCode.Decompiler/CSharp/SequencePointBuilder.cs

@@ -165,18 +165,54 @@ namespace ICSharpCode.Decompiler.CSharp
 			}
 		}
 
+		public override void VisitFieldDeclaration(FieldDeclaration fieldDeclaration)
+		{
+			foreach (var variable in fieldDeclaration.Variables)
+			{
+				VisitMemberInitializer(variable.Initializer);
+			}
+			base.VisitFieldDeclaration(fieldDeclaration);
+		}
+
 		public override void VisitPropertyDeclaration(PropertyDeclaration propertyDeclaration)
 		{
 			if (propertyDeclaration.ExpressionBody is not null)
 			{
 				VisitAsSequencePoint(propertyDeclaration.ExpressionBody);
 			}
+			else if (propertyDeclaration.Initializer is not null)
+			{
+				VisitMemberInitializer(propertyDeclaration.Initializer);
+			}
 			else
 			{
 				base.VisitPropertyDeclaration(propertyDeclaration);
 			}
 		}
 
+		// A field/auto-property/event initializer is emitted once at the declaration but, in IL,
+		// runs in every instance constructor that does not chain to this(...) (and static
+		// initializers run in the static constructor). The primary sequence point covers the
+		// constructor the initializer was lifted from; MemberInitializerInOtherConstructorsAnnotation
+		// carries the initializer's copies from the remaining constructors, so a matching sequence
+		// point is emitted - at the same source location, but with the other constructor's IL - for
+		// each of them.
+		void VisitMemberInitializer(Expression? initializer)
+		{
+			if (initializer is null)
+				return;
+			VisitAsSequencePoint(initializer);
+			var annotation = initializer.Annotation<MemberInitializerInOtherConstructorsAnnotation>();
+			if (annotation is null)
+				return;
+			foreach (var other in annotation.Initializers)
+			{
+				StartSequencePoint(initializer);
+				other.AcceptVisitor(this);
+				EndSequencePoint(initializer.StartLocation, initializer.EndLocation);
+			}
+		}
+
 		public override void VisitIndexerDeclaration(IndexerDeclaration indexerDeclaration)
 		{
 			if (indexerDeclaration.ExpressionBody is not null)
@@ -204,6 +240,15 @@ namespace ICSharpCode.Decompiler.CSharp
 			VisitAsSequencePoint(forStatement.EmbeddedStatement);
 		}
 
+		public override void VisitEventDeclaration(EventDeclaration eventDeclaration)
+		{
+			foreach (var variable in eventDeclaration.Variables)
+			{
+				VisitMemberInitializer(variable.Initializer);
+			}
+			base.VisitEventDeclaration(eventDeclaration);
+		}
+
 		public override void VisitSwitchStatement(SwitchStatement switchStatement)
 		{
 			StartSequencePoint(switchStatement);

ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs

@@ -110,7 +110,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			[AllowNull]
 			public List<(Statement Statement, IMember Member, Expression Initializer, bool DependsOnConstructorBody)> Statements;
 
-			public Dictionary<Statement, List<Statement>>? StatementToOtherCtorsMap;
+			public Dictionary<Statement, List<(Statement Statement, Expression Initializer)>>? StatementToOtherCtorsMap;
 
 			public bool HasDuplicateAssignments { get; private set; }
 			public bool IsUnsafe { get; private set; }
@@ -223,7 +223,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 						list = [];
 						StatementToOtherCtorsMap[stmt] = list;
 					}
-					list.Add(otherStmt);
+					list.Add((otherStmt, otherInitializer));
 					otherStmt = otherStmt.GetNextStatement();
 				}
 				return true;
@@ -576,11 +576,19 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 					stmt.Remove();
 
 					if (sequence.StatementToOtherCtorsMap != null &&
-						sequence.StatementToOtherCtorsMap.TryGetValue(stmt, out var otherStmts))
+						sequence.StatementToOtherCtorsMap.TryGetValue(stmt, out var otherCtors))
 					{
-						foreach (var otherStmt in otherStmts)
+						var otherInitializers = new List<Expression>(otherCtors.Count);
+						foreach (var (otherStmt, otherInitializer) in otherCtors)
 						{
 							otherStmt.Remove();
+							otherInitializers.Add(otherInitializer);
+						}
+						// Preserve the discarded copies so the breakpoint for this initializer can be
+						// emitted in every constructor that runs it, not just the one it was lifted from.
+						if (otherInitializers.Count > 0)
+						{
+							initializer.AddAnnotation(new MemberInitializerInOtherConstructorsAnnotation(otherInitializers));
 						}
 					}
 

ICSharpCode.Decompiler/DebugInfo/PortablePdbWriter.cs

@@ -203,6 +203,24 @@ namespace ICSharpCode.Decompiler.DebugInfo
 								metadata.GetOrAddBlob(function.AsyncDebugInfo.BuildBlob(methodHandle))));
 						}
 					}
+
+					// Sequence points are keyed by the ILFunction resolved through the IL instruction
+					// tree, which still resolves for member initializers that were lifted out of a
+					// constructor the decompiler omits from the output (an implicit default constructor,
+					// an implicit static constructor, or a primary constructor). DebugInfoGenerator only
+					// discovers functions through their declaration syntax, so those constructors are not
+					// in its set; emit their sequence points here so the lifted initializers stay
+					// breakpointable.
+					var emittedFunctions = new HashSet<ILFunction>(debugInfoGen.Functions);
+					foreach (var (function, points) in sequencePoints)
+					{
+						if (!emittedFunctions.Add(function))
+							continue;
+						var method = function.MoveNextMethod ?? function.Method;
+						if (method == null || method.MetadataToken.IsNil)
+							continue;
+						ProcessMethod((MethodDefinitionHandle)method.MetadataToken, document, points, syntaxTree);
+					}
 				}
 			}