Fix local function called from method and state machine

docs/error-codes.md

@@ -1827,6 +1827,37 @@ void TestMethod()
   }
   ```
 
+#### `IL2117`: Trim analysis: Methods 'method1' and 'method2' are both associated with lambda or local function 'method'. This is currently unsupported and may lead to incorrectly reported warnings.
+
+- Trimmer currently can't correctly handle if the same compiler generated lambda or local function is associated with two different methods. We don't know of any C# patterns which would cause this problem, but it is possible to write code like this in IL.
+
+  Only a meta-sample:
+
+  ```C#
+  [RequiresUnreferencedCode ("")] // This should suppress all warnings from the method
+  void UserDefinedMethod()
+  {
+      // Uses the compiler-generated local function method
+      // The IL2026 from the local function should be suppressed in this case
+  }
+
+  // IL2107: Methods 'UserDefinedMethod' and 'SecondUserDefinedMethod' are both associated with state machine type '<compiler_generated_state_machine>_type'.
+  [RequiresUnreferencedCode ("")] // This should suppress all warnings from the method
+  void SecondUserDefinedMethod()
+  {
+      // Uses the compiler-generated local function method
+      // The IL2026 from the local function should be suppressed in this case
+  }
+
+  internal static void <UserDefinedMethod>g__LocalFunction|0_0()
+  {
+      // Compiler-generated method emitted for a local function.
+      // This should only ever be called from one user-defined method.
+  }
+
+  ```
+
+
 ## Single-File Warning Codes
 
 #### `IL3000`: 'member' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'

src/ILLink.Shared/DiagnosticId.cs

@@ -175,6 +175,7 @@ public enum DiagnosticId
 		DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers = 2114,
 		DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers = 2115,
 		RequiresUnreferencedCodeOnStaticConstructor = 2116,
+		MethodsAreAssociatedWithUserMethod = 2117,
 
 		// Single-file diagnostic ids.
 		AvoidAssemblyLocationInSingleFile = 3000,

src/ILLink.Shared/SharedStrings.resx

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <root>
   <!--
     Microsoft ResX Schema
@@ -1065,6 +1065,12 @@
   <data name="RequiresUnreferencedCodeOnStaticConstructorMessage" xml:space="preserve">
     <value>'RequiresUnreferencedCodeAttribute' cannot be placed directly on static constructor '{0}', consider placing 'RequiresUnreferencedCodeAttribute' on the type declaration instead.</value>
   </data>
+  <data name="MethodsAreAssociatedWithUserMethodTitle" xml:space="preserve">
+    <value>Trimmer currently can't correctly handle if the same compiler generated lambda or local function is associated with two different methods.</value>
+  </data>
+  <data name="MethodsAreAssociatedWithUserMethodMessage" xml:space="preserve">
+    <value>Methods '{0}' and '{1}' are both associated with lambda or local function '{2}'. This is currently unsupported and may lead to incorrectly reported warnings.</value>
+  </data>
   <data name="AvoidAssemblyLocationInSingleFileTitle" xml:space="preserve">
     <value>Avoid accessing Assembly file path when publishing as a single file</value>
   </data>

src/linker/Linker/CompilerGeneratedCallGraph.cs

@@ -0,0 +1,63 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using Mono.Cecil;
+
+namespace Mono.Linker
+{
+	class CompilerGeneratedCallGraph
+	{
+		readonly Dictionary<IMemberDefinition, HashSet<IMemberDefinition>> callGraph;
+
+		public CompilerGeneratedCallGraph () => callGraph = new Dictionary<IMemberDefinition, HashSet<IMemberDefinition>> ();
+
+		void TrackCallInternal (IMemberDefinition fromMember, IMemberDefinition toMember)
+		{
+			if (!callGraph.TryGetValue (fromMember, out HashSet<IMemberDefinition>? toMembers)) {
+				toMembers = new HashSet<IMemberDefinition> ();
+				callGraph.Add (fromMember, toMembers);
+			}
+			toMembers.Add (toMember);
+		}
+
+		public void TrackCall (MethodDefinition fromMethod, MethodDefinition toMethod)
+		{
+			Debug.Assert (CompilerGeneratedNames.IsLambdaOrLocalFunction (toMethod.Name));
+			TrackCallInternal (fromMethod, toMethod);
+		}
+
+		public void TrackCall (MethodDefinition fromMethod, TypeDefinition toType)
+		{
+			Debug.Assert (CompilerGeneratedNames.IsStateMachineType (toType.Name));
+			TrackCallInternal (fromMethod, toType);
+		}
+
+		public void TrackCall (TypeDefinition fromType, MethodDefinition toMethod)
+		{
+			Debug.Assert (CompilerGeneratedNames.IsStateMachineType (fromType.Name));
+			Debug.Assert (CompilerGeneratedNames.IsLambdaOrLocalFunction (toMethod.Name));
+			TrackCallInternal (fromType, toMethod);
+		}
+
+		public IEnumerable<IMemberDefinition> GetReachableMembers (MethodDefinition start)
+		{
+			Queue<IMemberDefinition> queue = new ();
+			HashSet<IMemberDefinition> visited = new ();
+			visited.Add (start);
+			queue.Enqueue (start);
+			while (queue.TryDequeue (out IMemberDefinition? method)) {
+				if (!callGraph.TryGetValue (method, out HashSet<IMemberDefinition>? callees))
+					continue;
+
+				foreach (var callee in callees) {
+					if (visited.Add (callee)) {
+						queue.Enqueue (callee);
+						yield return callee;
+					}
+				}
+			}
+		}
+	}
+}

src/linker/Linker/CompilerGeneratedNames.cs

@@ -20,6 +20,18 @@ internal static bool IsLambdaDisplayClass (string className)
 			return className.StartsWith ("<>c");
 		}
 
+		internal static bool IsStateMachineType (string typeName)
+		{
+			if (!IsGeneratedMemberName (typeName))
+				return false;
+
+			int i = typeName.LastIndexOf ('>');
+			if (i == -1)
+				return false;
+
+			return (typeName.Length > i + 1) && typeName[i + 1] == 'd';
+		}
+
 		internal static bool IsLambdaOrLocalFunction (string methodName) => IsLambdaMethod (methodName) || IsLocalFunction (methodName);
 
 		// Lambda methods have generated names like "<UserMethod>b__0_1" where "UserMethod" is the name

src/linker/Linker/CompilerGeneratedState.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
@@ -45,15 +46,22 @@ void PopulateCacheForType (TypeDefinition type)
 			if (!_typesWithPopulatedCache.Add (type))
 				return;
 
-			var callGraph = new CallGraph ();
+			var callGraph = new CompilerGeneratedCallGraph ();
 			var callingMethods = new HashSet<MethodDefinition> ();
 
 			void ProcessMethod (MethodDefinition method)
 			{
+				bool isStateMachineMember = CompilerGeneratedNames.IsStateMachineType (method.DeclaringType.Name);
 				if (!CompilerGeneratedNames.IsLambdaOrLocalFunction (method.Name)) {
-					// If it's not a nested function, track as an entry point to the call graph.
-					var added = callingMethods.Add (method);
-					Debug.Assert (added);
+					if (!isStateMachineMember) {
+						// If it's not a nested function, track as an entry point to the call graph.
+						var added = callingMethods.Add (method);
+						Debug.Assert (added);
+					}
+				} else {
+					// We don't expect lambdas or local functions to be emitted directly into
+					// state machine types.
+					Debug.Assert (!isStateMachineMember);
 				}
 
 				// Discover calls or references to lambdas or local functions. This includes
@@ -70,7 +78,11 @@ void ProcessMethod (MethodDefinition method)
 						if (!CompilerGeneratedNames.IsLambdaOrLocalFunction (lambdaOrLocalFunction.Name))
 							continue;
 
-						callGraph.TrackCall (method, lambdaOrLocalFunction);
+						if (isStateMachineMember) {
+							callGraph.TrackCall (method.DeclaringType, lambdaOrLocalFunction);
+						} else {
+							callGraph.TrackCall (method, lambdaOrLocalFunction);
+						}
 					}
 				}
 
@@ -92,6 +104,7 @@ void ProcessMethod (MethodDefinition method)
 						Debug.Assert (stateMachineType.DeclaringType == type ||
 							(CompilerGeneratedNames.IsGeneratedMemberName (stateMachineType.DeclaringType.Name) &&
 							 stateMachineType.DeclaringType.DeclaringType == type));
+						callGraph.TrackCall (method, stateMachineType);
 						if (!_compilerGeneratedTypeToUserCodeMethod.TryAdd (stateMachineType, method)) {
 							var alreadyAssociatedMethod = _compilerGeneratedTypeToUserCodeMethod[stateMachineType];
 							_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithStateMachine, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), stateMachineType.GetDisplayName ());
@@ -129,9 +142,27 @@ void ProcessMethod (MethodDefinition method)
 			// code or its referenced nested functions. There is no reliable way to determine from
 			// IL which user code an unused nested function belongs to.
 			foreach (var userDefinedMethod in callingMethods) {
-				foreach (var nestedFunction in callGraph.GetReachableMethods (userDefinedMethod)) {
-					Debug.Assert (CompilerGeneratedNames.IsLambdaOrLocalFunction (nestedFunction.Name));
-					_compilerGeneratedMethodToUserCodeMethod.Add (nestedFunction, userDefinedMethod);
+				foreach (var compilerGeneratedMember in callGraph.GetReachableMembers (userDefinedMethod)) {
+					switch (compilerGeneratedMember) {
+					case MethodDefinition nestedFunction:
+						Debug.Assert (CompilerGeneratedNames.IsLambdaOrLocalFunction (nestedFunction.Name));
+						// Nested functions get suppressions from the user method only.
+						if (!_compilerGeneratedMethodToUserCodeMethod.TryAdd (nestedFunction, userDefinedMethod)) {
+							var alreadyAssociatedMethod = _compilerGeneratedMethodToUserCodeMethod[nestedFunction];
+							_context.LogWarning (new MessageOrigin (userDefinedMethod), DiagnosticId.MethodsAreAssociatedWithUserMethod, userDefinedMethod.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), nestedFunction.GetDisplayName ());
+						}
+						break;
+					case TypeDefinition stateMachineType:
+						// Types in the call graph are always state machine types
+						// For those all their methods are not tracked explicitly in the call graph; instead, they
+						// are represented by the state machine type itself.
+						// We are already tracking the association of the state machine type to the user code method
+						// above, so no need to track it here.
+						Debug.Assert (CompilerGeneratedNames.IsStateMachineType (stateMachineType.Name));
+						break;
+					default:
+						throw new InvalidOperationException ();
+					}
 				}
 			}
 		}

test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs

@@ -123,13 +123,13 @@ public class F
 ";
 
 			await VerifyRequiresDynamicCodeCodeFix (test, fixtest, new[] {
-				// /0/Test0.cs(9,17): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+				// /0/Test0.cs(9,17): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 				VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(9, 17, 9, 21).WithArguments("C.M1()", " message.", ""),
-				// /0/Test0.cs(13,27): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+				// /0/Test0.cs(13,27): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 				VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(13, 27, 13, 33).WithArguments("C.M1()", " message.", ""),
-				// /0/Test0.cs(17,31): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+				// /0/Test0.cs(17,31): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 				VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(17, 31, 17, 37).WithArguments("C.M1()", " message.", ""),
-				// /0/Test0.cs(24,31): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+				// /0/Test0.cs(24,31): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 				VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(24, 31, 24, 37).WithArguments("C.M1()", " message.", "")
 			}, new[] {
 				// /0/Test0.cs(27,10): error CS7036: There is no argument given that corresponds to the required formal parameter 'message' of 'RequiresDynamicCodeAttribute.RequiresDynamicCodeAttribute(string)'
@@ -176,7 +176,7 @@ Action M2()
 				src,
 				fix,
 				baselineExpected: new[] {
-					// /0/Test0.cs(12,22): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+					// /0/Test0.cs(12,22): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 					VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(12, 22, 12, 26).WithArguments("C.M1()", " message.", "")
 				},
 				fixedExpected: Array.Empty<DiagnosticResult> ());
@@ -221,7 +221,7 @@ Action M2()
 				src,
 				fix,
 				baselineExpected: new[] {
-					// /0/Test0.cs(12,28): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+					// /0/Test0.cs(12,28): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 					VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(12, 28, 12, 32).WithArguments("C.M1()", " message.", "")
 				},
 				fixedExpected: Array.Empty<DiagnosticResult> (),
@@ -262,7 +262,7 @@ public class C
 				src,
 				fix,
 				baselineExpected: new[] {
-					// /0/Test0.cs(10,19): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+					// /0/Test0.cs(10,19): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 					VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(10, 19, 10, 23).WithArguments("C.M1()", " message.", "")
 				},
 				fixedExpected: new[] {
@@ -301,11 +301,11 @@ public class C
 				src,
 				fix,
 				baselineExpected: new[] {
-					// /0/Test0.cs(10,15): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+					// /0/Test0.cs(10,15): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 					VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(10, 15, 10, 19).WithArguments("C.M1()", " message.", "")
 				},
 				fixedExpected: new[] {
-					// /0/Test0.cs(10,15): warning IL2117: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
+					// /0/Test0.cs(10,15): warning IL3050: Using member 'C.M1()' which has 'RequiresDynamicCodeAttribute' can break functionality when trimming application code. message.
 					VerifyCS.Diagnostic(DiagnosticId.RequiresDynamicCode).WithSpan(10, 15, 10, 19).WithArguments("C.M1()", " message.", "")
 				});
 		}

test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs

@@ -184,7 +184,7 @@ public class C
 				test,
 				fixtest,
 				baselineExpected: new[] {
-				// /0/Test0.cs(7,17): warning IL2117: Members annotated with 'RequiresDynamicCodeAttribute' require dynamic access otherwise can break functionality when trimming application code. message.
+				// /0/Test0.cs(7,17): warning IL3050: Members annotated with 'RequiresDynamicCodeAttribute' require dynamic access otherwise can break functionality when trimming application code. message.
 				VerifyCSUSMwithRDC.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (7, 17, 7, 21).WithArguments ("C.M1()", " message.", "")
 				},
 				fixedExpected: Array.Empty<DiagnosticResult> ());