diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs index 1a29c7948abf7..4b546b6217834 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs @@ -24,18 +24,12 @@ public CapturedReferenceValue (IOperation operation) case OperationKind.InlineArrayAccess: case OperationKind.ImplicitIndexerReference: break; - case OperationKind.None: case OperationKind.InstanceReference: case OperationKind.Invocation: - case OperationKind.Invalid: // These will just be ignored when referenced later. break; default: - // Assert on anything else as it means we need to implement support for it - // but do not throw here as it means new Roslyn version could cause the analyzer to crash - // which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do - // so effectively ignoring constructs it doesn't understand is OK. - Debug.Fail ($"{operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}"); + UnexpectedOperationHandler.Handle (operation); break; } Reference = operation; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index 33e7da7b634fd..4968379af5b12 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -335,9 +335,6 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe // Seems like this can't happen with a flow capture operation. Debug.Assert (operation.Target is not IFlowCaptureReferenceOperation); break; - case IInvalidOperation: - // This can happen for a field assignment in an attribute instance. - // TODO: validate against the field attributes. case IInstanceReferenceOperation: // Assignment to 'this' is not tracked currently. // Not relevant for trimming dataflow. @@ -358,16 +355,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe // can show up in a flow capture reference (for example, where the right-hand side // is a null-coalescing operator). default: - // NoneOperation represents operations which are unimplemented by Roslyn - // (don't have specific I*Operation types), such as pointer dereferences. - if (targetOperation.Kind is OperationKind.None) - break; - - // Assert on anything else as it means we need to implement support for it - // but do not throw here as it means new Roslyn version could cause the analyzer to crash - // which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do - // so effectively ignoring constructs it doesn't understand is OK. - Debug.Fail ($"{targetOperation.GetType ()}: {targetOperation.Syntax.GetLocation ().GetLineSpan ()}"); + UnexpectedOperationHandler.Handle (targetOperation); break; } return Visit (operation.Value, state); @@ -584,9 +572,7 @@ public override TValue VisitDelegateCreation (IDelegateCreationOperation operati // No method symbol. break; default: - // Unimplemented case that might need special handling. - // Fail in debug mode only. - Debug.Fail ($"{operation.Target.GetType ()}: {operation.Target.Syntax.GetLocation ().GetLineSpan ()}"); + UnexpectedOperationHandler.Handle (operation.Target); break; } @@ -781,7 +767,7 @@ TValue HandleMethodCallHelper ( argumentOperation = callOperation.Arguments[argumentIndex]; break; default: - Debug.Fail ($"Unexpected operation {operation} for parameter {parameter.GetDisplayName ()}"); + UnexpectedOperationHandler.Handle (operation); continue; }; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/UnexpectedOperationHandler.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/UnexpectedOperationHandler.cs new file mode 100644 index 0000000000000..bb661d5aac8f4 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/UnexpectedOperationHandler.cs @@ -0,0 +1,50 @@ +// 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.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.CodeAnalysis +{ + internal static class UnexpectedOperationHandler + { + // No-op in release builds, but fails in debug builds when + // encountering an unexpected operation. InvalidOperation is skipped because + // it is expected that any part of the control-flow graph may contain an + // InvalidOperation for code that doesn't compile (for example, in an intermediate + // state while editing). + [Conditional ("DEBUG")] + public static void Handle (IOperation operation) + { + // NoneOperation represents operations which are unimplemented by Roslyn + // (don't have specific I*Operation types), such as pointer dereferences. + if (operation.Kind is OperationKind.None) + return; + + if (operation.Kind is OperationKind.Invalid) + return; + + // It's also possible to hit a case where the operation is an unexpected operation kind, + // but the code is in an invalid state where the unexpected operation is not IInvalidOperation, yet one + // of its child operations is. For example: + // + // a + = 3; + // + // This is represented as an assignment where the target is an IBinaryOperation (a +) whose right-hand side + // is an IInvalidOperation. The assignment logic doesn't support assigning to a binary operation, + // but this should still not fail. + foreach (var descendant in operation.Descendants()) { + if (descendant.Kind is OperationKind.Invalid) + return; + } + + // Throw on anything else as it means we need to implement support for it + // but do not throw in Release builds as it means new Roslyn version could cause the analyzer to crash + // which is not fixable by the user. The analyzer is not going to be 100% correct no + // matter what we do so effectively ignoring constructs it doesn't understand is OK. + // This is surfaced as warning AD0001 in Debug builds. + throw new NotImplementedException ($"Unexpected operation type {operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}"); + } + } +} diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs index 0b7f9e5d1c8ba..3194dde84bda5 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs @@ -1384,6 +1384,25 @@ public static void Main() DiagnosticResult.CompilerError ("CS0103").WithSpan (8, 3, 8, 7).WithArguments ("type")); } + [Fact] + public Task AssignmentTargetHasNestedInvalidOperation () + { + // The assignment target is an IBinaryOperation whose right-hand side is an IInvalidOperation. + var Source = $$""" + int a, b = 0; + a + = 3; + """; + + return VerifyDynamicallyAccessedMembersAnalyzer (Source, consoleApplication: true, + // (2,6): error CS1525: Invalid expression term '=' + DiagnosticResult.CompilerError("CS1525").WithSpan(2, 6, 2, 7).WithArguments("="), + // (2,2): error CS0165: Use of unassigned local variable 'a' + DiagnosticResult.CompilerError("CS0165").WithSpan(2, 2, 2, 3).WithArguments("a"), + // (1,9): warning CS0219: The variable 'b' is assigned but its value is never used + DiagnosticResult.CompilerWarning("CS0219").WithSpan(1, 9, 1, 10).WithArguments("b") + ); + } + [Fact] public Task CRefGenericParameterAnalysis () { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributeFieldDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributeFieldDataflow.cs index d826e2e446db7..e581797e23474 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributeFieldDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributeFieldDataflow.cs @@ -12,20 +12,56 @@ namespace Mono.Linker.Tests.Cases.DataFlow [ExpectedNoWarnings] class AttributeFieldDataflow { + public static void Main () + { + TestKeepsPublicConstructors (); + TestKeepsPublicMethods (); + TestKeepsPublicMethodsString (); + TestKeepsPublicFields (); + TestTypeArray (); + } + + [Kept] [KeptAttributeAttribute (typeof (KeepsPublicConstructorsAttribute))] + [KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))] + public static void TestKeepsPublicConstructors () + { + typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicConstructors)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute)); + } + + [Kept] [KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))] + [ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--")] + [KeepsPublicMethods (Type = typeof (ClassWithKeptPublicMethods))] + public static void TestKeepsPublicMethods () + { + typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicMethods)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute)); + } + + [Kept] + [KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))] + // Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273 + [ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributeFieldDataflow+ClassWithKeptPublicMethods")] + public static void TestKeepsPublicMethodsString () + { + typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicMethodsString)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute)); + } + + [Kept] [KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))] - [KeptAttributeAttribute (typeof (TypeArrayAttribute))] - [KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))] - [KeepsPublicMethods (Type = "Mono.Linker.Tests.Cases.DataFlow.AttributeFieldDataflow+ClassWithKeptPublicMethods")] [KeepsPublicFields (Type = null, TypeName = null)] + public static void TestKeepsPublicFields () + { + typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicFields)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute)); + } + + [Kept] + [KeptAttributeAttribute (typeof (TypeArrayAttribute))] [TypeArray (Types = new Type[] { typeof (AttributeFieldDataflow) })] - // Trimmer only for now - https://github.com/dotnet/linker/issues/2273 - [ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - public static void Main () + public static void TestTypeArray () { - typeof (AttributeFieldDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute)); - typeof (AttributeFieldDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute)); + typeof (AttributeFieldDataflow).GetMethod (nameof (TestTypeArray)).GetCustomAttribute (typeof (TypeArrayAttribute)); } [Kept] @@ -55,7 +91,12 @@ public KeepsPublicMethodsAttribute () [Kept] [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] - public string Type; + public string TypeName; + + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + public Type Type; } // Use to test null values diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs index fd5fce1b7238e..46034848c4953 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs @@ -27,20 +27,56 @@ public static void Main () class AttributesOnMethod { + [Kept] + public static void Test () { + TestKeepsPublicConstructors (); + TestKeepsPublicMethods (); + TestKeepsPublicMethodsByName (); + TestKeepsPublicFields (); + TestTypeArray (); + } + [Kept] [KeptAttributeAttribute (typeof (KeepsPublicConstructorsAttribute))] + [KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))] + public static void TestKeepsPublicConstructors () + { + typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicConstructors)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute)); + } + + [Kept] + [KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))] + [ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--")] + [KeepsPublicMethods (Type = typeof (ClassWithKeptPublicMethods))] + public static void TestKeepsPublicMethods () + { + typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicMethods)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute)); + } + + [Kept] [KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))] + // Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273 + [ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethodsKeptByName--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] + [KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+AttributesOnMethod+ClassWithKeptPublicMethodsKeptByName")] + public static void TestKeepsPublicMethodsByName () + { + typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicMethodsByName)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute)); + } + + [Kept] [KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))] - [KeptAttributeAttribute (typeof (TypeArrayAttribute))] - [KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))] - [KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+AttributesOnMethod+ClassWithKeptPublicMethods")] [KeepsPublicFields (Type = null, TypeName = null)] + public static void TestKeepsPublicFields () + { + typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicFields)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute)); + } + + [Kept] + [KeptAttributeAttribute (typeof (TypeArrayAttribute))] [TypeArray (Types = new Type[] { typeof (AttributePropertyDataflow) })] - // Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273 - [ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)] - public static void Test () { - typeof (AttributesOnMethod).GetMethod ("Test").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute)); - typeof (AttributePropertyDataflow).GetMethod ("Test").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute)); + public static void TestTypeArray () + { + typeof (AttributesOnMethod).GetMethod (nameof (TestTypeArray)).GetCustomAttribute (typeof (TypeArrayAttribute)); } [Kept] @@ -63,6 +99,16 @@ class ClassWithKeptPublicMethods public static void KeptMethod () { } static void Method () { } } + + [Kept] + class ClassWithKeptPublicMethodsKeptByName + { + [Kept] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode ("--ClassWithKeptPublicMethodsKeptByName--")] + public static void KeptMethod () { } + static void Method () { } + } } class AttributesOnField