Map type parameters for static closures (#2899)

This discovers type parameter mappings between generic type
parameters and static closures which reference them. It detects
the closure environment by scanning for stsfld instructions which
reference generic closure types. There may be multiple methods
associated with the same static closure, so this case doesn't
produce warnings.

This also fixes an unrelated issue where in Release mode, the
compiler can generate struct closure environments that will never
get instantiated via a ctor call. A new testcase runs the
CompilerGeneratedTypes tests in Release mode to cover this.

6 files changed, 205 insertions, 48 deletions

diff --git a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs
index c99968f1d..3213f49fd 100644
--- a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs
+++ b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs
@@ -140,33 +140,58 @@ namespace Mono.Linker.Dataflow
 				// calls to local functions, and lambda assignments (which use ldftn).
 				if (method.Body != null) {
 					foreach (var instruction in method.Body.Instructions) {
-						if (instruction.OpCode.OperandType != OperandType.InlineMethod)
-							continue;
-
-						MethodDefinition? lambdaOrLocalFunction = _context.TryResolve ((MethodReference) instruction.Operand);
-						if (lambdaOrLocalFunction == null)
-							continue;
-
-						if (lambdaOrLocalFunction.IsConstructor &&
-							lambdaOrLocalFunction.DeclaringType is var generatedType &&
-							// Don't consider calls in the same type, like inside a static constructor
-							method.DeclaringType != generatedType &&
-							CompilerGeneratedNames.IsLambdaDisplayClass (generatedType.Name)) {
-							// fill in null for now, attribute providers will be filled in later
-							if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, new TypeArgumentInfo (method, null))) {
-								var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod;
-								_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
-							}
-							continue;
-						}
+						switch (instruction.OpCode.OperandType) {
+						case OperandType.InlineMethod: {
+								MethodDefinition? referencedMethod = _context.TryResolve ((MethodReference) instruction.Operand);
+								if (referencedMethod == null)
+									continue;
+
+								if (referencedMethod.IsConstructor &&
+									referencedMethod.DeclaringType is var generatedType &&
+									// Don't consider calls in the same type, like inside a static constructor
+									method.DeclaringType != generatedType &&
+									CompilerGeneratedNames.IsLambdaDisplayClass (generatedType.Name)) {
+									// fill in null for now, attribute providers will be filled in later
+									if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, new TypeArgumentInfo (method, null))) {
+										var alreadyAssociatedMethod = _generatedTypeToTypeArgumentInfo[generatedType].CreatingMethod;
+										_context.LogWarning (new MessageOrigin (method), DiagnosticId.MethodsAreAssociatedWithUserMethod, method.GetDisplayName (), alreadyAssociatedMethod.GetDisplayName (), generatedType.GetDisplayName ());
+									}
+									continue;
+								}
 
-						if (!CompilerGeneratedNames.IsLambdaOrLocalFunction (lambdaOrLocalFunction.Name))
-							continue;
+								if (!CompilerGeneratedNames.IsLambdaOrLocalFunction (referencedMethod.Name))
+									continue;
 
-						if (isStateMachineMember) {
-							callGraph.TrackCall (method.DeclaringType, lambdaOrLocalFunction);
-						} else {
-							callGraph.TrackCall (method, lambdaOrLocalFunction);
+								if (isStateMachineMember) {
+									callGraph.TrackCall (method.DeclaringType, referencedMethod);
+								} else {
+									callGraph.TrackCall (method, referencedMethod);
+								}
+							}
+							break;
+
+						case OperandType.InlineField: {
+								// Same as above, but stsfld instead of a call to the constructor
+								if (instruction.OpCode.Code is not Code.Stsfld)
+									continue;
+
+								FieldDefinition? field = _context.TryResolve ((FieldReference) instruction.Operand);
+								if (field == null)
+									continue;
+
+								if (field.DeclaringType is var generatedType &&
+									// Don't consider field accesses in the same type, like inside a static constructor
+									method.DeclaringType != generatedType &&
+									CompilerGeneratedNames.IsLambdaDisplayClass (generatedType.Name)) {
+									if (!_generatedTypeToTypeArgumentInfo.TryAdd (generatedType, new TypeArgumentInfo (method, null))) {
+										// It's expected that there may be multiple methods associated with the same static closure environment.
+										// All of these methods will substitute the same type arguments into the closure environment
+										// (if it is generic). Don't warn.
+									}
+									continue;
+								}
+							}
+							break;
 						}
 					}
 				}
@@ -274,14 +299,7 @@ namespace Mono.Linker.Dataflow
 			{
 				Debug.Assert (CompilerGeneratedNames.IsGeneratedType (generatedType.Name));
 
-				if (!_generatedTypeToTypeArgumentInfo.TryGetValue (generatedType, out var typeInfo)) {
-					// This can happen for static (non-capturing) closure environments, where more than
-					// nested function can map to the same closure environment. Since the current functionality
-					// is based on a one-to-one relationship between environments (types) and methods, this is
-					// not supported.
-					return;
-				}
-
+				var typeInfo = _generatedTypeToTypeArgumentInfo[generatedType];
 				if (typeInfo.OriginalAttributes is not null) {
 					return;
 				}
@@ -312,9 +330,10 @@ namespace Mono.Linker.Dataflow
 									userAttrs = param;
 								} else if (_context.TryResolve ((TypeReference) param.Owner) is { } owningType) {
 									MapGeneratedTypeTypeParameters (owningType);
-									if (_generatedTypeToTypeArgumentInfo.TryGetValue (owningType, out var owningInfo) &&
-										owningInfo.OriginalAttributes is { } owningAttrs) {
+									if (_generatedTypeToTypeArgumentInfo[owningType].OriginalAttributes is { } owningAttrs) {
 										userAttrs = owningAttrs[param.Position];
+									} else {
+										Debug.Assert (false, "This should be impossible in valid code");
 									}
 								}
 							}
@@ -327,18 +346,41 @@ namespace Mono.Linker.Dataflow
 				}
 			}
 
-			GenericInstanceType? ScanForInit (TypeDefinition stateMachineType, MethodBody body)
+			GenericInstanceType? ScanForInit (TypeDefinition compilerGeneratedType, MethodBody body)
 			{
 				foreach (var instr in body.Instructions) {
+					bool handled = false;
 					switch (instr.OpCode.Code) {
 					case Code.Initobj:
-					case Code.Newobj:
-						if (instr.Operand is MethodReference { DeclaringType: GenericInstanceType typeRef }
-							&& stateMachineType == _context.TryResolve (typeRef)) {
-							return typeRef;
+					case Code.Newobj: {
+							if (instr.Operand is MethodReference { DeclaringType: GenericInstanceType typeRef }
+								&& compilerGeneratedType == _context.TryResolve (typeRef)) {
+								return typeRef;
+							}
+							handled = true;
+						}
+						break;
+					case Code.Stsfld: {
+							if (instr.Operand is FieldReference { DeclaringType: GenericInstanceType typeRef }
+								&& compilerGeneratedType == _context.TryResolve (typeRef)) {
+								return typeRef;
+							}
+							handled = true;
 						}
 						break;
 					}
+
+					// Also look for type substitutions into generic methods
+					// (such as AsyncTaskMethodBuilder::Start<TStateMachine>).
+					if (!handled && instr.OpCode.OperandType is OperandType.InlineMethod) {
+						if (instr.Operand is GenericInstanceMethod gim) {
+							foreach (var tr in gim.GenericArguments) {
+								if (tr is GenericInstanceType git && compilerGeneratedType == _context.TryResolve (git)) {
+									return git;
+								}
+							}
+						}
+					}
 				}
 				return null;
 			}
diff --git a/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs
index 73a0260f5..7cbf0e69c 100644
--- a/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs
+++ b/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs
@@ -71,6 +71,12 @@ namespace ILLink.RoslynAnalyzer.Tests
 		}
 
 		[Fact]
+		public Task CompilerGeneratedTypesRelease ()
+		{
+			return RunTest ();
+		}
+
+		[Fact]
 		public Task ComplexTypeHandling ()
 		{
 			return RunTest ();
diff --git a/test/Mono.Linker.Tests.Cases/Attributes/OnlyKeepUsed/NullableOnConstraints.cs b/test/Mono.Linker.Tests.Cases/Attributes/OnlyKeepUsed/NullableOnConstraints.cs
index 3cfb0bcd9..b396bb3b1 100644
--- a/test/Mono.Linker.Tests.Cases/Attributes/OnlyKeepUsed/NullableOnConstraints.cs
+++ b/test/Mono.Linker.Tests.Cases/Attributes/OnlyKeepUsed/NullableOnConstraints.cs
@@ -6,7 +6,6 @@ using Mono.Linker.Tests.Cases.Expectations.Metadata;
 namespace Mono.Linker.Tests.Cases.Attributes.OnlyKeepUsed
 {
 	[SetupCSharpCompilerToUse ("csc")]
-	[SetupCompileArgument ("/langversion:8.0")]
 	[SetupLinkerArgument ("--used-attrs-only", "true")]
 	public class NullableOnConstraints
 	{
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs
index d4a24e1bc..97791146d 100644
--- a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypes.cs
@@ -12,7 +12,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 {
 	[ExpectedNoWarnings]
 	[SkipKeptItemsValidation]
-	class CompilerGeneratedTypes
+	public class CompilerGeneratedTypes
 	{
 		public static void Main ()
 		{
@@ -34,6 +34,12 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 
 			// Closures
 			GlobalClosures ();
+			LambdaInsideIterator<int> ();
+			LocalFunctionInsideIterator<int> ();
+			CapturingLambdaInsideIterator<int> ();
+			CapturingLocalFunctionInsideIterator<int> ();
+			LambdaInsideAsync<int> ();
+			LocalFunctionInsideAsync<int> ();
 		}
 
 		private static void UseIterator ()
@@ -237,6 +243,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 		{
 			GlobalClosureClass<int>.M1<int> ();
 			GlobalClosureClass<int>.M2<int> ();
+			GlobalClosureClass<int>.SameClosureForLambda1 ();
+			GlobalClosureClass<int>.SameClosureForLambda2 ();
 		}
 
 		private sealed class GlobalClosureClass<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T>
@@ -245,8 +253,6 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 			{
 				Func<string, Action> a = (string s) => () => Console.WriteLine (s + typeof (T).GetMethods ());
 				Func<string, Action> b = (string s) =>
-					// https://github.com/dotnet/linker/issues/2826
-					[ExpectedWarning ("IL2090", "U", "PublicProperties", ProducedBy = ProducedBy.Trimmer)]
 				() => Console.WriteLine (s + typeof (U).GetProperties ());
 				a ("");
 				b ("");
@@ -255,12 +261,77 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 			{
 				Func<string, Action> a = (string s) => () => Console.WriteLine (s + typeof (T).GetMethods ());
 				Func<string, Action> b = (string s) =>
-					// https://github.com/dotnet/linker/issues/2826
-					[ExpectedWarning ("IL2090", "U", "PublicProperties", ProducedBy = ProducedBy.Trimmer)]
 				() => Console.WriteLine (s + typeof (U).GetProperties ());
 				a ("");
 				b ("");
 			}
+
+			public static void SameClosureForLambda1 ()
+			{
+				var l =
+				[ExpectedWarning ("IL2090", nameof (GlobalClosureClass<T>), nameof (System.Type.GetProperties))]
+				() => typeof (T).GetProperties ();
+				l ();
+			}
+
+			public static void SameClosureForLambda2 ()
+			{
+				var l =
+				[ExpectedWarning ("IL2090", nameof (GlobalClosureClass<T>), nameof (System.Type.GetProperties))]
+				() => typeof (T).GetProperties ();
+				l ();
+			}
+		}
+
+		static IEnumerable<int> LambdaInsideIterator<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ()
+		{
+			var l = () => typeof (T).GetMethods ();
+			yield return 1;
+			l ();
+		}
+
+		static IEnumerable<int> LocalFunctionInsideIterator<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ()
+		{
+			void LocalFunction () => typeof (T).GetMethods ();
+			yield return 1;
+			LocalFunction ();
+		}
+
+		static IEnumerable<int> CapturingLambdaInsideIterator<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ()
+		{
+			int i = 0;
+			var l = () => {
+				typeof (T).GetMethods ();
+				i++;
+			};
+			yield return i;
+			l ();
+		}
+
+		static IEnumerable<int> CapturingLocalFunctionInsideIterator<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ()
+		{
+			int i = 0;
+			void LocalFunction ()
+			{
+				typeof (T).GetMethods ();
+				i++;
+			}
+			yield return i;
+			LocalFunction ();
+		}
+
+		static async Task LambdaInsideAsync<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ()
+		{
+			var l = () => typeof (T).GetMethods ();
+			await Task.Delay (0);
+			l ();
+		}
+
+		static async Task LocalFunctionInsideAsync<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ()
+		{
+			void LocalFunction () => typeof (T).GetMethods ();
+			await Task.Delay (0);
+			LocalFunction ();
 		}
 	}
 }
\ No newline at end of file
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypesRelease.cs b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypesRelease.cs
new file mode 100644
index 000000000..f852fa541
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedTypesRelease.cs
@@ -0,0 +1,29 @@
+// 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.CodeAnalysis;
+using System.Reflection;
+using System.Threading.Tasks;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.DataFlow
+{
+	[ExpectedNoWarnings]
+	[SkipKeptItemsValidation]
+	[SetupCompileArgument ("/optimize+")]
+	[SetupCompileArgument ("/main:Mono.Linker.Tests.Cases.DataFlow.CompilerGeneratedTypesRelease")]
+	[SandboxDependency ("CompilerGeneratedTypes.cs")]
+	class CompilerGeneratedTypesRelease
+	{
+		// This test just links the CompilerGeneratedTypes test in the Release configuration, to test
+		// different compilation strategies for closures and state machine types.
+		// Sometimes the compiler produces classes in Debug mode and structs in Release mode.
+		public static void Main ()
+		{
+			CompilerGeneratedTypes.Main ();
+		}
+	}
+}
\ No newline at end of file
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs
index 51d534be4..e41c2b8e7 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// 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;
@@ -251,6 +251,9 @@ namespace Mono.Linker.Tests.TestCasesRunner
 					case "/optimize+":
 						compilationOptions = compilationOptions.WithOptimizationLevel (OptimizationLevel.Release);
 						break;
+					case "/optimize-":
+						compilationOptions = compilationOptions.WithOptimizationLevel (OptimizationLevel.Debug);
+						break;
 					case "/debug:full":
 					case "/debug:pdbonly":
 						// Use platform's default debug info. This behavior is the same as csc.
@@ -267,7 +270,14 @@ namespace Mono.Linker.Tests.TestCasesRunner
 					case "/langversion:7.3":
 						languageVersion = LanguageVersion.CSharp7_3;
 						break;
-
+					default:
+						var splitIndex = option.IndexOf (":");
+						if (splitIndex != -1 && option[..splitIndex] == "/main") {
+							var mainTypeName = option[(splitIndex + 1)..];
+							compilationOptions = compilationOptions.WithMainTypeName (mainTypeName);
+							break;
+						}
+						throw new NotImplementedException (option);
 					}
 				}
 			}