Propagate DynamicallyAccessedMemberTypes through Nullable<T> to T (#2675

)

Add intrinsic support for Nullable.GetUnderlyingType and support for MakeGenericType with Nullables

Adds ArrayCreationOperation visitors to create ArrayValue's in the analyzer, and adds start of dataflow analysis for array values.

Adds tests to validate dataflow in Arrays.

Co-authored-by: vitek-karas <[email protected]>

src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs

@@ -66,7 +66,9 @@ public void Transfer (BlockProxy block, LocalDataFlowState<TValue, TValueLattice
 
 		public abstract void HandleAssignment (TValue source, TValue target, IOperation operation);
 
-		public abstract TValue HandleArrayElementAccess (IOperation arrayReference);
+		public abstract TValue HandleArrayElementRead (TValue arrayValue, TValue indexValue, IOperation operation);
+
+		public abstract void HandleArrayElementWrite (TValue arrayValue, TValue indexValue, TValue valueToWrite, IOperation operation);
 
 		// This takes an IOperation rather than an IReturnOperation because the return value
 		// may (must?) come from BranchValue of an operation whose FallThroughSuccessor is the exit block.
@@ -112,8 +114,11 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
 					operation);
 				break;
 			// TODO: when setting a property in an attribute, target is an IPropertyReference.
-			case IArrayElementReferenceOperation:
-				// TODO
+			case IArrayElementReferenceOperation arrayElementRef:
+				if (arrayElementRef.Indices.Length != 1)
+					break;
+
+				HandleArrayElementWrite (Visit (arrayElementRef.ArrayReference, state), Visit (arrayElementRef.Indices[0], state), value, operation);
 				break;
 			case IDiscardOperation:
 				// Assignments like "_ = SomeMethod();" don't need dataflow tracking.
@@ -194,7 +199,12 @@ public override TValue VisitArrayElementReference (IArrayElementReferenceOperati
 			if (operation.GetValueUsageInfo (Context.OwningSymbol).HasFlag (ValueUsageInfo.Read)) {
 				// Accessing an array element for reading is a call to the indexer
 				// or a plain array access. Just handle plain array access for now.
-				return HandleArrayElementAccess (operation.ArrayReference);
+
+				// Only handle simple index access
+				if (operation.Indices.Length != 1)
+					return TopValue;
+
+				return HandleArrayElementRead (Visit (operation.ArrayReference, state), Visit (operation.Indices[0], state), operation);
 			}
 
 			return TopValue;

src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs

@@ -10,7 +10,6 @@
 using ILLink.Shared;
 using ILLink.Shared.DataFlow;
 using ILLink.Shared.TrimAnalysis;
-using ILLink.Shared.TypeSystemProxy;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -165,28 +164,14 @@ static void ProcessGenericParameters (SyntaxNodeAnalysisContext context)
 					// These uninstantiated generics should not produce warnings.
 					if (typeArgs[i].Kind == SymbolKind.ErrorType)
 						continue;
-					var sourceValue = GetTypeValueNodeFromGenericArgument (context, typeArgs[i]);
+					var sourceValue = SingleValueExtensions.FromTypeSymbol (typeArgs[i])!;
 					var targetValue = new GenericParameterValue (typeParams[i]);
 					foreach (var diagnostic in GetDynamicallyAccessedMembersDiagnostics (sourceValue, targetValue, context.Node.GetLocation ()))
 						context.ReportDiagnostic (diagnostic);
 				}
 			}
 		}
 
-		static SingleValue GetTypeValueNodeFromGenericArgument (SyntaxNodeAnalysisContext context, ITypeSymbol type)
-		{
-			return type.Kind switch {
-				SymbolKind.TypeParameter => new GenericParameterValue ((ITypeParameterSymbol) type),
-				// Technically this should be a new value node type as it's not a System.Type instance representation, but just the generic parameter
-				// That said we only use it to perform the dynamically accessed members checks and for that purpose treating it as System.Type is perfectly valid.
-				SymbolKind.NamedType => new SystemTypeValue (new TypeProxy ((INamedTypeSymbol) type)),
-				SymbolKind.ErrorType => UnknownValue.Instance,
-				SymbolKind.ArrayType => new SystemTypeValue (new TypeProxy (context.Compilation.GetTypeByMetadataName ("System.Array")!)),
-				// What about things like PointerType and so on. Linker treats these as "named types" since it can resolve them to concrete type
-				_ => throw new NotImplementedException ()
-			};
-		}
-
 		static IEnumerable<Diagnostic> GetDynamicallyAccessedMembersDiagnostics (SingleValue sourceValue, SingleValue targetValue, Location location)
 		{
 			// The target should always be an annotated value, but the visitor design currently prevents

src/ILLink.RoslynAnalyzer/TrimAnalysis/ArrayValue.cs

@@ -1,18 +1,71 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
+using System.Collections.Generic;
 using ILLink.Shared.DataFlow;
 using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>;
 
 namespace ILLink.Shared.TrimAnalysis
 {
 	partial record ArrayValue
 	{
-		public ArrayValue (SingleValue size) => Size = size;
+		public readonly Dictionary<int, MultiValue> IndexValues;
 
-#pragma warning disable IDE0060 // Remove unused parameter
-		public partial bool TryGetValueByIndex (int index, out MultiValue value) => throw new NotImplementedException ();
-#pragma warning restore IDE0060 // Remove unused parameter
+		public static MultiValue Create (MultiValue size)
+		{
+			MultiValue result = MultiValueLattice.Top;
+			foreach (var sizeValue in size) {
+				result = MultiValueLattice.Meet (result, new MultiValue (new ArrayValue (sizeValue)));
+			}
+
+			return result;
+		}
+
+		ArrayValue (SingleValue size)
+		{
+			Size = size;
+			IndexValues = new Dictionary<int, MultiValue> ();
+		}
+
+		public partial bool TryGetValueByIndex (int index, out MultiValue value)
+		{
+			if (IndexValues.TryGetValue (index, out value))
+				return true;
+
+			value = default;
+			return false;
+		}
+
+		public override int GetHashCode ()
+		{
+			return HashUtils.Combine (GetType ().GetHashCode (), Size);
+		}
+
+		public bool Equals (ArrayValue? otherArr)
+		{
+			if (otherArr == null)
+				return false;
+
+			bool equals = Size.Equals (otherArr.Size);
+			equals &= IndexValues.Count == otherArr.IndexValues.Count;
+			if (!equals)
+				return false;
+
+			// If both sets T and O are the same size and "T intersect O" is empty, then T == O.
+			HashSet<KeyValuePair<int, MultiValue>> thisValueSet = new (IndexValues);
+			thisValueSet.ExceptWith (otherArr.IndexValues);
+			return thisValueSet.Count == 0;
+		}
+
+		// Lattice Meet() is supposed to copy values, so we need to make a deep copy since ArrayValue is mutable through IndexValues
+		public override SingleValue DeepCopy ()
+		{
+			var newArray = new ArrayValue (Size);
+			foreach (var kvp in IndexValues) {
+				newArray.IndexValues.Add (kvp.Key, kvp.Value.Clone ());
+			}
+
+			return newArray;
+		}
 	}
-}
+}

src/ILLink.RoslynAnalyzer/TrimAnalysis/FieldValue.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using ILLink.RoslynAnalyzer;
+using ILLink.Shared.DataFlow;
 using Microsoft.CodeAnalysis;
 
 namespace ILLink.Shared.TrimAnalysis
@@ -19,6 +20,8 @@ partial record FieldValue
 		public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch ()
 			=> new string[] { FieldSymbol.GetDisplayName () };
 
+		public override SingleValue DeepCopy () => this; // This value is immutable
+
 		public override string ToString () => this.ValueToString (FieldSymbol, DynamicallyAccessedMemberTypes);
 	}
 }

src/ILLink.RoslynAnalyzer/TrimAnalysis/GenericParameterValue.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using ILLink.RoslynAnalyzer;
+using ILLink.Shared.DataFlow;
 using Microsoft.CodeAnalysis;
 
 namespace ILLink.Shared.TrimAnalysis
@@ -26,6 +27,8 @@ public partial bool HasDefaultConstructorConstraint () =>
 		public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch ()
 			=> new string[] { GenericParameter.TypeParameterSymbol.Name, GenericParameter.TypeParameterSymbol.ContainingSymbol.GetDisplayName () };
 
+		public override SingleValue DeepCopy () => this; // This value is immutable
+
 		public override string ToString ()
 			=> this.ValueToString (GenericParameter.TypeParameterSymbol, DynamicallyAccessedMemberTypes);
 	}

src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs

@@ -5,6 +5,7 @@
 using System.Diagnostics.CodeAnalysis;
 using ILLink.RoslynAnalyzer;
 using ILLink.RoslynAnalyzer.TrimAnalysis;
+using ILLink.Shared.DataFlow;
 using Microsoft.CodeAnalysis;
 
 namespace ILLink.Shared.TrimAnalysis
@@ -24,6 +25,8 @@ public MethodParameterValue (IParameterSymbol parameterSymbol, DynamicallyAccess
 		public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch ()
 			=> new string[] { ParameterSymbol.GetDisplayName (), ParameterSymbol.ContainingSymbol.GetDisplayName () };
 
+		public override SingleValue DeepCopy () => this; // This value is immutable
+
 		public override string ToString ()
 			=> this.ValueToString (ParameterSymbol, DynamicallyAccessedMemberTypes);
 	}

src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodReturnValue.cs

@@ -5,6 +5,7 @@
 using System.Diagnostics.CodeAnalysis;
 using ILLink.RoslynAnalyzer;
 using ILLink.RoslynAnalyzer.TrimAnalysis;
+using ILLink.Shared.DataFlow;
 using Microsoft.CodeAnalysis;
 
 namespace ILLink.Shared.TrimAnalysis
@@ -30,6 +31,8 @@ public MethodReturnValue (IMethodSymbol methodSymbol, DynamicallyAccessedMemberT
 		public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch ()
 			=> new string[] { MethodSymbol.GetDisplayName () };
 
+		public override SingleValue DeepCopy () => this; // This value is immutable
+
 		public override string ToString () => this.ValueToString (MethodSymbol, DynamicallyAccessedMemberTypes);
 	}
 }

src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodThisParameterValue.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using ILLink.RoslynAnalyzer;
+using ILLink.Shared.DataFlow;
 using Microsoft.CodeAnalysis;
 
 namespace ILLink.Shared.TrimAnalysis
@@ -23,6 +24,8 @@ public MethodThisParameterValue (IMethodSymbol methodSymbol, DynamicallyAccessed
 		public override IEnumerable<string> GetDiagnosticArgumentsForAnnotationMismatch ()
 			=> new string[] { MethodSymbol.GetDisplayName () };
 
+		public override SingleValue DeepCopy () => this; // This value is immutable
+
 		public override string ToString () => this.ValueToString (MethodSymbol, DynamicallyAccessedMemberTypes);
 	}
 }

src/ILLink.RoslynAnalyzer/TrimAnalysis/RuntimeMethodHandleValue.cs

@@ -0,0 +1,14 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using ILLink.Shared.DataFlow;
+
+namespace ILLink.Shared.TrimAnalysis
+{
+	partial record RuntimeMethodHandleValue
+	{
+		public override SingleValue DeepCopy () => this; // immutable value
+
+		public override string ToString () => this.ValueToString ();
+	}
+}

src/ILLink.RoslynAnalyzer/TrimAnalysis/SingleValueExtensions.cs

@@ -0,0 +1,40 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Linq;
+using ILLink.Shared.DataFlow;
+using ILLink.Shared.TrimAnalysis;
+using ILLink.Shared.TypeSystemProxy;
+using Microsoft.CodeAnalysis;
+
+namespace ILLink.RoslynAnalyzer.TrimAnalysis
+{
+	public static class SingleValueExtensions
+	{
+		public static SingleValue? FromTypeSymbol (ITypeSymbol type)
+		{
+			if (type.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T) {
+				var underlyingType = (type as INamedTypeSymbol)?.TypeArguments.FirstOrDefault ();
+				return underlyingType?.TypeKind switch {
+					TypeKind.TypeParameter =>
+						new NullableValueWithDynamicallyAccessedMembers (new TypeProxy (type),
+							new GenericParameterValue ((ITypeParameterSymbol) underlyingType)),
+					// typeof(Nullable<>) 
+					TypeKind.Error => new SystemTypeValue (new TypeProxy (type)),
+					TypeKind.Class or TypeKind.Struct or TypeKind.Interface =>
+						new NullableSystemTypeValue (new TypeProxy (type), new SystemTypeValue (new TypeProxy (underlyingType))),
+					_ => UnknownValue.Instance
+				};
+			}
+			return type.Kind switch {
+				SymbolKind.TypeParameter => new GenericParameterValue ((ITypeParameterSymbol) type),
+				SymbolKind.NamedType => new SystemTypeValue (new TypeProxy (type)),
+				// If the symbol is an Array type, the BaseType is System.Array
+				SymbolKind.ArrayType => new SystemTypeValue (new TypeProxy (type.BaseType!)),
+				SymbolKind.ErrorType => UnknownValue.Instance,
+				_ => null
+			};
+
+		}
+	}
+}