From a58df95ba140373930cb71be4c0475b0b1644deb Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Sat, 18 Nov 2023 00:28:55 +0000 Subject: [PATCH 01/10] Add analyzer support for feature guards --- .../DataFlow/FeatureChecksValue.cs | 9 +- ...heckVisitor.cs => FeatureChecksVisitor.cs} | 53 ++- .../DataFlow/LocalDataFlowVisitor.cs | 17 +- ...rContext.cs => DataFlowAnalyzerContext.cs} | 0 .../DynamicallyAccessedMembersAnalyzer.cs | 3 + .../ISymbolExtensions.cs | 42 ++ .../RequiresAnalyzerBase.cs | 27 +- .../RequiresAssemblyFilesAnalyzer.cs | 6 +- .../RequiresDynamicCodeAnalyzer.cs | 6 +- .../RequiresUnreferencedCodeAnalyzer.cs | 8 +- .../TrimAnalysisAssignmentPattern.cs | 2 +- .../TrimAnalysisMethodCallPattern.cs | 2 +- .../TrimAnalysis/TrimAnalysisPatternStore.cs | 21 +- .../TrimAnalysisReflectionAccessPattern.cs | 2 +- .../TrimAnalysisReturnValuePattern.cs.cs | 61 +++ .../TrimAnalysis/TrimAnalysisVisitor.cs | 29 +- .../illink/src/ILLink.Shared/DiagnosticId.cs | 4 + .../src/ILLink.Shared/SharedStrings.resx | 14 +- .../DataFlowTests.cs | 6 + .../Support/FeatureGuardAttribute.cs | 17 + .../DataFlow/Dependencies/TestFeatures.cs | 12 + .../DataFlow/FeatureCheckDataFlow.cs | 20 +- .../FeatureCheckDataFlowTestSubstitutions.xml | 2 +- .../DataFlow/FeatureGuardDataFlow.cs | 441 ++++++++++++++++++ .../FeatureGuardDataFlowTestSubstitutions.xml | 42 ++ 25 files changed, 772 insertions(+), 74 deletions(-) rename src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/{FeatureCheckVisitor.cs => FeatureChecksVisitor.cs} (64%) rename src/tools/illink/src/ILLink.RoslynAnalyzer/{DataflowAnalyzerContext.cs => DataFlowAnalyzerContext.cs} (100%) create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs index 268833431274a0..086e16bddfcd37 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs @@ -13,11 +13,13 @@ namespace ILLink.RoslynAnalyzer.DataFlow // For now, this is only designed to track the built-in "features"/"capabilities" // like RuntimeFeatures.IsDynamicCodeSupported, where a true return value // indicates that a feature/capability is available. - public record struct FeatureChecksValue : INegate + public record struct FeatureChecksValue : INegate, IDeepCopyValue { public ValueSet EnabledFeatures; public ValueSet DisabledFeatures; + public static FeatureChecksValue None = new FeatureChecksValue (ValueSet.Empty, ValueSet.Empty); + public FeatureChecksValue (string enabledFeature) { EnabledFeatures = new ValueSet (enabledFeature); @@ -48,5 +50,10 @@ public FeatureChecksValue Negate () { return new FeatureChecksValue (DisabledFeatures.DeepCopy (), EnabledFeatures.DeepCopy ()); } + + public FeatureChecksValue DeepCopy () + { + return new FeatureChecksValue (EnabledFeatures.DeepCopy (), DisabledFeatures.DeepCopy ()); + } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureCheckVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs similarity index 64% rename from src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureCheckVisitor.cs rename to src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs index 707294718fda32..a8bf871072ddfd 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureCheckVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs @@ -24,7 +24,7 @@ namespace ILLink.RoslynAnalyzer.DataFlow // (a set features that are checked to be enabled or disabled). // The visitor takes a LocalDataFlowState as an argument, allowing for checks that // depend on the current dataflow state. - public class FeatureChecksVisitor : OperationVisitor + public class FeatureChecksVisitor : OperationVisitor { DataFlowAnalyzerContext _dataFlowAnalyzerContext; @@ -33,32 +33,39 @@ public FeatureChecksVisitor (DataFlowAnalyzerContext dataFlowAnalyzerContext) _dataFlowAnalyzerContext = dataFlowAnalyzerContext; } - public override FeatureChecksValue? VisitArgument (IArgumentOperation operation, StateValue state) + public override FeatureChecksValue DefaultVisit (IOperation operation, StateValue state) + { + // Visiting a non-understood pattern should return the empty set of features, which will + // prevent this check from acting as a guard for any feature. + return FeatureChecksValue.None; + } + + public override FeatureChecksValue VisitArgument (IArgumentOperation operation, StateValue state) { return Visit (operation.Value, state); } - public override FeatureChecksValue? VisitPropertyReference (IPropertyReferenceOperation operation, StateValue state) + public override FeatureChecksValue VisitPropertyReference (IPropertyReferenceOperation operation, StateValue state) { + // A single property may serve as a feature check for multiple features. + FeatureChecksValue featureChecks = FeatureChecksValue.None; foreach (var analyzer in _dataFlowAnalyzerContext.EnabledRequiresAnalyzers) { - if (analyzer.IsRequiresCheck (_dataFlowAnalyzerContext.Compilation, operation.Property)) { - return new FeatureChecksValue (analyzer.FeatureName); + if (analyzer.IsFeatureGuard (operation.Property, _dataFlowAnalyzerContext)) { + var featureCheck = new FeatureChecksValue (analyzer.RequiresAttributeFullyQualifiedName); + featureChecks = featureChecks.And (featureCheck); } } - return null; + return featureChecks; } - public override FeatureChecksValue? VisitUnaryOperator (IUnaryOperation operation, StateValue state) + public override FeatureChecksValue VisitUnaryOperator (IUnaryOperation operation, StateValue state) { if (operation.OperatorKind is not UnaryOperatorKind.Not) - return null; + return FeatureChecksValue.None; - FeatureChecksValue? context = Visit (operation.Operand, state); - if (context == null) - return null; - - return context.Value.Negate (); + FeatureChecksValue context = Visit (operation.Operand, state); + return context.Negate (); } public bool? GetLiteralBool (IOperation operation) @@ -77,7 +84,7 @@ public FeatureChecksVisitor (DataFlowAnalyzerContext dataFlowAnalyzerContext) return value; } - public override FeatureChecksValue? VisitBinaryOperator (IBinaryOperation operation, StateValue state) + public override FeatureChecksValue VisitBinaryOperator (IBinaryOperation operation, StateValue state) { bool expectEqual; switch (operation.OperatorKind) { @@ -88,36 +95,32 @@ public FeatureChecksVisitor (DataFlowAnalyzerContext dataFlowAnalyzerContext) expectEqual = false; break; default: - return null; + return FeatureChecksValue.None; } if (GetLiteralBool (operation.LeftOperand) is bool leftBool) { - if (Visit (operation.RightOperand, state) is not FeatureChecksValue rightValue) - return null; + FeatureChecksValue rightValue = Visit (operation.RightOperand, state); return leftBool == expectEqual ? rightValue : rightValue.Negate (); } if (GetLiteralBool (operation.RightOperand) is bool rightBool) { - if (Visit (operation.LeftOperand, state) is not FeatureChecksValue leftValue) - return null; + FeatureChecksValue leftValue = Visit (operation.LeftOperand, state); return rightBool == expectEqual ? leftValue : leftValue.Negate (); } - return null; + return FeatureChecksValue.None; } - public override FeatureChecksValue? VisitIsPattern (IIsPatternOperation operation, StateValue state) + public override FeatureChecksValue VisitIsPattern (IIsPatternOperation operation, StateValue state) { if (GetExpectedValueFromPattern (operation.Pattern) is not bool patternValue) - return null; - - if (Visit (operation.Value, state) is not FeatureChecksValue value) - return null; + return FeatureChecksValue.None; + FeatureChecksValue value = Visit (operation.Value, state); return patternValue ? value : value.Negate (); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index 33e7da7b634fdc..63c89c5073d0c8 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -88,12 +88,12 @@ public LocalDataFlowVisitor ( return null; var branchValue = Visit (branchValueOperation, state); - + TConditionValue conditionValue = GetConditionValue (branchValueOperation, state); if (block.Block.ConditionKind != ControlFlowConditionKind.None) { // BranchValue may represent a value used in a conditional branch to the ConditionalSuccessor. // If so, give the analysis an opportunity to model the checked condition, and return the model // of the condition back to the generic analysis. It will be applied to the state of each outgoing branch. - return GetConditionValue (branchValueOperation, state); + return conditionValue; } // If not, the BranchValue represents a return or throw value associated with the FallThroughSuccessor of this block. @@ -118,10 +118,13 @@ public LocalDataFlowVisitor ( // We don't want the return operation because this might have multiple possible return values in general. var current = state.Current; HandleReturnValue (branchValue, branchValueOperation, in current.Context); + // Must be called for every return value even if it did not return an understood condition, + // because the non-understood conditions will produce warnings for FeatureGuard properties. + HandleReturnConditionValue (conditionValue, branchValueOperation); return null; } - public abstract TConditionValue? GetConditionValue ( + public abstract TConditionValue GetConditionValue ( IOperation branchValueOperation, LocalDataFlowState state); @@ -146,6 +149,10 @@ public abstract void HandleReturnValue ( IOperation operation, in TContext context); + public abstract void HandleReturnConditionValue ( + TConditionValue returnConditionValue, + IOperation branchValueOperation); + // This is called for any method call, which includes: // - Normal invocation operation // - Accessing property value - which is treated as a call to the getter @@ -787,9 +794,7 @@ TValue HandleMethodCallHelper ( // Get the condition value that is being asserted. If the attribute is DoesNotReturnIf(true), // the condition value needs to be negated so that we can assert the false condition. - if (GetConditionValue (argumentOperation, state) is not TConditionValue conditionValue) - continue; - + TConditionValue conditionValue = GetConditionValue (argumentOperation, state); var current = state.Current; ApplyCondition ( doesNotReturnIfConditionValue == false diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlowAnalyzerContext.cs similarity index 100% rename from src/tools/illink/src/ILLink.RoslynAnalyzer/DataflowAnalyzerContext.cs rename to src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlowAnalyzerContext.cs diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index 231b418f814f71..f7d77cd4b227d4 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -25,6 +25,7 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute); public const string attributeArgument = "attributeArgument"; public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute; + public const string FullyQualifiedFeatureGuardAttribute = "System.Diagnostics.CodeAnalysis.FeatureGuardAttribute"; public static Lazy> RequiresAnalyzers { get; } = new Lazy> (GetRequiresAnalyzers); static ImmutableArray GetRequiresAnalyzers () => ImmutableArray.Create ( @@ -55,6 +56,8 @@ public static ImmutableArray GetSupportedDiagnostics () diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedTypeNameInTypeGetType)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedParameterInMethodCreateInstance)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ParametersOfAssemblyCreateInstanceCannotBeAnalyzed)); + diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ReturnValueDoesNotMatchFeatureGuards)); + diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.InvalidFeatureGuard)); foreach (var requiresAnalyzer in RequiresAnalyzers.Value) { foreach (var diagnosticDescriptor in requiresAnalyzer.SupportedDiagnostics) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs index 7e830f7c6ecd4c..03d6a8cbe5c138 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs @@ -1,9 +1,14 @@ // 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.Immutable; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Text; using Microsoft.CodeAnalysis; +using ILLink.RoslynAnalyzer.DataFlow; +using ILLink.Shared.DataFlow; namespace ILLink.RoslynAnalyzer { @@ -34,6 +39,14 @@ internal static bool TryGetAttribute (this ISymbol member, string attributeName, return false; } + internal static IEnumerable GetAttributes (this ISymbol member, string attributeName) + { + foreach (var attr in member.GetAttributes ()) { + if (attr.AttributeClass is { } attrClass && attrClass.HasName (attributeName)) + yield return attr; + } + } + internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes (this ISymbol symbol) { if (!TryGetAttribute (symbol, DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var dynamicallyAccessedMembers)) @@ -58,6 +71,35 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes return (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers.ConstructorArguments[0].Value!; } + internal static ValueSet GetFeatureGuardAnnotations ( + this IPropertySymbol propertySymbol, + IEnumerable enabledRequiresAnalyzers) + { + ImmutableArray.Builder featureSet = ImmutableArray.CreateBuilder (); + foreach (var attributeData in propertySymbol.GetAttributes ()) { + if (IsFeatureGuardAttribute (attributeData, out string? featureName)) + featureSet.Add (featureName); + } + return featureSet.Count == 0 ? ValueSet.Empty : new ValueSet (featureSet); + + bool IsFeatureGuardAttribute (AttributeData attributeData, [NotNullWhen (true)] out string? featureName) { + featureName = null; + if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureGuardAttribute)) + return false; + + if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureType }]) + return false; + + foreach (var analyzer in enabledRequiresAnalyzers) { + if (featureType.HasName (analyzer.RequiresAttributeFullyQualifiedName)) { + featureName = analyzer.RequiresAttributeFullyQualifiedName; + return true; + } + } + return false; + } + } + internal static bool TryGetReturnAttribute (this IMethodSymbol member, string attributeName, [NotNullWhen (returnValue: true)] out AttributeData? attribute) { attribute = null; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index d951404845cd41..9b441b67060384 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -5,8 +5,9 @@ using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Linq; -using ILLink.Shared; using ILLink.RoslynAnalyzer.DataFlow; +using ILLink.Shared; +using ILLink.Shared.DataFlow; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -19,9 +20,7 @@ public abstract class RequiresAnalyzerBase : DiagnosticAnalyzer { private protected abstract string RequiresAttributeName { get; } - internal abstract string FeatureName { get; } - - private protected abstract string RequiresAttributeFullyQualifiedName { get; } + internal abstract string RequiresAttributeFullyQualifiedName { get; } private protected abstract DiagnosticTargets AnalyzerDiagnosticTargets { get; } @@ -301,7 +300,23 @@ protected virtual bool CreateSpecialIncompatibleMembersDiagnostic ( // - false return value indicating that a feature is supported // - feature settings supplied by the project // - custom feature checks defined in library code - internal virtual bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) => false; + private protected virtual bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) => false; + + internal static bool IsAnnotatedFeatureGuard (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext, string featureName) + { + // Only respect FeatureGuardAttribute on static boolean properties. + if (!propertySymbol.IsStatic || propertySymbol.Type.SpecialType != SpecialType.System_Boolean) + return false; + + ValueSet featureGuards = propertySymbol.GetFeatureGuardAnnotations (dataFlowAnalyzerContext.EnabledRequiresAnalyzers); + return featureGuards.Contains (featureName); + } + + internal bool IsFeatureGuard (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext) + { + return IsAnnotatedFeatureGuard (propertySymbol, dataFlowAnalyzerContext, RequiresAttributeFullyQualifiedName) + || IsRequiresCheck (propertySymbol, dataFlowAnalyzerContext.Compilation); + } internal bool CheckAndCreateRequiresDiagnostic ( IOperation operation, @@ -312,7 +327,7 @@ internal bool CheckAndCreateRequiresDiagnostic ( [NotNullWhen (true)] out Diagnostic? diagnostic) { // Warnings are not emitted if the featureContext says the feature is available. - if (featureContext.IsEnabled (FeatureName)) { + if (featureContext.IsEnabled (RequiresAttributeFullyQualifiedName)) { diagnostic = null; return false; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs index e8807896d93735..8949b249b35ecb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs @@ -36,9 +36,7 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase private protected override string RequiresAttributeName => RequiresAssemblyFilesAttribute; - internal override string FeatureName => "AssemblyFiles"; - - private protected override string RequiresAttributeFullyQualifiedName => RequiresAssemblyFilesAttributeFullyQualifiedName; + internal override string RequiresAttributeFullyQualifiedName => RequiresAssemblyFilesAttributeFullyQualifiedName; private protected override DiagnosticTargets AnalyzerDiagnosticTargets => DiagnosticTargets.MethodOrConstructor | DiagnosticTargets.Property | DiagnosticTargets.Event; @@ -61,7 +59,7 @@ internal override bool IsAnalyzerEnabled (AnalyzerOptions options) return true; } - internal override bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { // "IsAssemblyFilesSupported" is treated as a requires check for testing purposes only, and // is not officially-supported product behavior. diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs index 5232ca9a9854e3..34bb7808d20314 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs @@ -25,9 +25,7 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase private protected override string RequiresAttributeName => RequiresDynamicCodeAttribute; - internal override string FeatureName => "DynamicCode"; - - private protected override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresDynamicCodeAttribute; + internal override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresDynamicCodeAttribute; private protected override DiagnosticTargets AnalyzerDiagnosticTargets => DiagnosticTargets.MethodOrConstructor | DiagnosticTargets.Class; @@ -40,7 +38,7 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer); - internal override bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) { + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { var runtimeFeaturesType = compilation.GetTypeByMetadataName ("System.Runtime.CompilerServices.RuntimeFeature"); if (runtimeFeaturesType == null) return false; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs index 3623150b752057..69c38629c43e17 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs @@ -49,11 +49,7 @@ private Action typeDerivesFromRucBase { private protected override string RequiresAttributeName => RequiresUnreferencedCodeAttribute; - public const string UnreferencedCode = nameof (UnreferencedCode); - - internal override string FeatureName => UnreferencedCode; - - private protected override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresUnreferencedCodeAttribute; + internal override string RequiresAttributeFullyQualifiedName => FullyQualifiedRequiresUnreferencedCodeAttribute; private protected override DiagnosticTargets AnalyzerDiagnosticTargets => DiagnosticTargets.MethodOrConstructor | DiagnosticTargets.Class; @@ -66,7 +62,7 @@ private Action typeDerivesFromRucBase { internal override bool IsAnalyzerEnabled (AnalyzerOptions options) => options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer); - internal override bool IsRequiresCheck (Compilation compilation, IPropertySymbol propertySymbol) + private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) { // "IsUnreferencedCodeSupported" is treated as a requires check for testing purposes only, and // is not officially-supported product behavior. diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs index 18046016db7ce1..86b995db4e4dd9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs @@ -58,7 +58,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) { + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { foreach (var sourceValue in Source) { foreach (var targetValue in Target) { // The target should always be an annotated value, but the visitor design currently prevents diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs index 7730ded1da3116..b7f882d996400f 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs @@ -77,7 +77,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope(out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { TrimAnalysisVisitor.HandleCall(Operation, OwningSymbol, CalledMethod, Instance, Arguments, diagnosticContext, default, out var _); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index d957ec61ad63c0..41f14398fdf2d9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -16,15 +16,19 @@ public readonly struct TrimAnalysisPatternStore readonly Dictionary FieldAccessPatterns; readonly Dictionary MethodCallPatterns; readonly Dictionary ReflectionAccessPatterns; + readonly Dictionary ReturnValuePatterns; readonly ValueSetLattice Lattice; readonly FeatureContextLattice FeatureContextLattice; - public TrimAnalysisPatternStore (ValueSetLattice lattice, FeatureContextLattice featureContextLattice) + public TrimAnalysisPatternStore ( + ValueSetLattice lattice, + FeatureContextLattice featureContextLattice) { AssignmentPatterns = new Dictionary<(IOperation, bool), TrimAnalysisAssignmentPattern> (); FieldAccessPatterns = new Dictionary (); MethodCallPatterns = new Dictionary (); ReflectionAccessPatterns = new Dictionary (); + ReturnValuePatterns = new Dictionary (); Lattice = lattice; FeatureContextLattice = featureContextLattice; } @@ -77,6 +81,16 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) ReflectionAccessPatterns[pattern.Operation] = pattern.Merge (Lattice, FeatureContextLattice, existingPattern); } + public void Add (TrimAnalysisReturnValuePattern pattern) + { + if (!ReturnValuePatterns.TryGetValue (pattern.Operation, out var existingPattern)) { + ReturnValuePatterns.Add (pattern.Operation, pattern); + return; + } + + Debug.Assert (existingPattern == pattern, "Return values should be identical"); + } + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { foreach (var assignmentPattern in AssignmentPatterns.Values) { @@ -98,6 +112,11 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte foreach (var diagnostic in reflectionAccessPattern.CollectDiagnostics (context)) yield return diagnostic; } + + foreach (var returnValuePattern in ReturnValuePatterns.Values) { + foreach (var diagnostic in returnValuePattern.CollectDiagnostics (context)) + yield return diagnostic; + } } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs index cfa6bfe1bde923..607ed75a517fec 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReflectionAccessPattern.cs @@ -50,7 +50,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ()); if (context.EnableTrimAnalyzer && !OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _) && - !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.UnreferencedCode)) { + !FeatureContext.IsEnabled (RequiresUnreferencedCodeAnalyzer.FullyQualifiedRequiresUnreferencedCodeAttribute)) { foreach (var diagnostic in ReflectionAccessAnalyzer.GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, ReferencedMethod)) diagnosticContext.AddDiagnostic (diagnostic); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs new file mode 100644 index 00000000000000..2dff971292a138 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs @@ -0,0 +1,61 @@ +// 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 ILLink.Shared; +using ILLink.Shared.DataFlow; +using ILLink.Shared.TrimAnalysis; +using ILLink.RoslynAnalyzer.DataFlow; +using Microsoft.CodeAnalysis; + +namespace ILLink.RoslynAnalyzer.TrimAnalysis +{ + public readonly record struct TrimAnalysisReturnValuePattern + { + public FeatureChecksValue ReturnValue { init; get; } + public ValueSet FeatureGuardAnnotations { init; get; } + public IOperation Operation { init; get; } + public IPropertySymbol OwningSymbol { init; get; } + + public TrimAnalysisReturnValuePattern ( + FeatureChecksValue returnValue, + ValueSet featureGuardAnnotations, + IOperation operation, + IPropertySymbol owningSymbol) + { + ReturnValue = returnValue.DeepCopy (); + FeatureGuardAnnotations = featureGuardAnnotations.DeepCopy (); + Operation = operation; + OwningSymbol = owningSymbol; + } + + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) + { + var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); + // For now, feature guard validation is enabled only when trim analysis is enabled. + if (context.EnableTrimAnalyzer) { + if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean) { + // Warn about invalid feature guards (non-static or non-bool properties) + diagnosticContext.AddDiagnostic ( + DiagnosticId.InvalidFeatureGuard); + return diagnosticContext.Diagnostics; + } + + ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; + // For any feature that this property is declared to guard, + // the abstract return value must include that feature + // (indicating it is known to be enabled when the return value is true). + foreach (string featureGuard in FeatureGuardAnnotations) { + if (!returnValueFeatures.Contains (featureGuard)) { + diagnosticContext.AddDiagnostic ( + DiagnosticId.ReturnValueDoesNotMatchFeatureGuards, + OwningSymbol.GetDisplayName (), + featureGuard); + } + } + } + + return diagnosticContext.Diagnostics; + } + } +} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 21fd3d77a72389..6eed9f640a4793 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -42,6 +42,8 @@ public class TrimAnalysisVisitor : LocalDataFlowVisitor< FeatureChecksVisitor _featureChecksVisitor; + DataFlowAnalyzerContext _dataFlowAnalyzerContext; + public TrimAnalysisVisitor ( Compilation compilation, LocalStateAndContextLattice, FeatureContextLattice> lattice, @@ -56,14 +58,15 @@ public TrimAnalysisVisitor ( _multiValueLattice = lattice.LocalStateLattice.Lattice.ValueLattice; TrimAnalysisPatterns = trimAnalysisPatterns; _featureChecksVisitor = new FeatureChecksVisitor (dataFlowAnalyzerContext); + _dataFlowAnalyzerContext = dataFlowAnalyzerContext; } - public override FeatureChecksValue? GetConditionValue (IOperation branchValueOperation, StateValue state) + public override FeatureChecksValue GetConditionValue (IOperation branchValueOperation, StateValue state) { return _featureChecksVisitor.Visit (branchValueOperation, state); } - public override void ApplyCondition (FeatureChecksValue featureChecksValue, ref LocalStateAndContext currentState) + public override void ApplyCondition (FeatureChecksValue featureChecksValue, ref LocalStateAndContext currentState) { currentState.Context = currentState.Context.Union (new FeatureContext (featureChecksValue.EnabledFeatures)); } @@ -421,6 +424,28 @@ public override void HandleReturnValue (MultiValue returnValue, IOperation opera } } + public override void HandleReturnConditionValue (FeatureChecksValue returnConditionValue, IOperation operation) + { + // Return statements should only happen inside of method bodies. + Debug.Assert (OwningSymbol is IMethodSymbol); + if (OwningSymbol is not IMethodSymbol method) + return; + + // FeatureGuard validation needs to happen only for properties. + if (method.MethodKind != MethodKind.PropertyGet) + return; + + IPropertySymbol propertySymbol = (IPropertySymbol) method.AssociatedSymbol!; + var featureGuards = propertySymbol.GetFeatureGuardAnnotations (_dataFlowAnalyzerContext.EnabledRequiresAnalyzers); + + // If there are no feature guards, there is nothing to validate. + if (featureGuards.IsEmpty()) + return; + + TrimAnalysisPatterns.Add ( + new TrimAnalysisReturnValuePattern (returnConditionValue, featureGuards, operation, propertySymbol)); + } + public override MultiValue HandleDelegateCreation (IMethodSymbol method, IOperation operation, FeatureContext featureContext) { TrimAnalysisPatterns.Add (new TrimAnalysisReflectionAccessPattern ( diff --git a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs index 1c33bb084a04af..5d40efbbaaf994 100644 --- a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs +++ b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs @@ -202,6 +202,10 @@ public enum DiagnosticId GenericRecursionCycle = 3054, CorrectnessOfAbstractDelegatesCannotBeGuaranteed = 3055, RequiresDynamicCodeOnStaticConstructor = 3056, + + // Feature guard diagnostic ids. + ReturnValueDoesNotMatchFeatureGuards = 4000, + InvalidFeatureGuard = 4001 } public static class DiagnosticIdExtensions diff --git a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx index 111c1c5877de7f..16d387cfca0e77 100644 --- a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx +++ b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx @@ -1197,4 +1197,16 @@ Unused 'UnconditionalSuppressMessageAttribute' found. Consider removing the unused warning suppression. - \ No newline at end of file + + Return value does not match FeatureGuardAttribute '{1}'. + + + Return value does not match feature guards of the property. + + + Invalid FeatureGuardAttribute. Ensure the attribute is plated on a static boolean property. + + + Invalid FeatureGuardAttribute. + + diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index e30551fa126cd3..1316c54903eb02 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -149,6 +149,12 @@ public Task FeatureCheckDataFlow () return RunTest (); } + [Fact] + public Task FeatureGuardDataFlow () + { + return RunTest (); + } + [Fact] public Task FieldDataFlow () { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs new file mode 100644 index 00000000000000..3ef6c5205ed55d --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + // Allow AttributeTargets.Method for testing invalid usages of a custom FeatureGuardAttribute + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Method, Inherited=false, AllowMultiple=true)] + public sealed class FeatureGuardAttribute : Attribute + { + public Type RequiresAttributeType { get; } + + public FeatureGuardAttribute(Type requiresAttributeType) + { + RequiresAttributeType = requiresAttributeType; + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs new file mode 100644 index 00000000000000..942c9f3586dd34 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/TestFeatures.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace ILLink.RoslynAnalyzer +{ + public class TestFeatures + { + public static bool IsUnreferencedCodeSupported => true; + + public static bool IsAssemblyFilesSupported => true; + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs index d6c29aa739cee8..adac4db80550a0 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlow.cs @@ -17,7 +17,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow // Note: the XML must be passed as an embedded resource named ILLink.Substitutions.xml, // not as a separate substitution file, for it to work with NativeAot. // Related: https://github.com/dotnet/runtime/issues/88647 - [SetupCompileResource ("FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml")] + [SetupCompileBefore ("TestFeatures.dll", new[] { "Dependencies/TestFeatures.cs" }, + resources: new object[] { new [] { "FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml" } })] [IgnoreSubstitutions (false)] public class FeatureCheckDataFlow { @@ -490,14 +491,14 @@ static void CallTestUnreferencedCodeUnguarded () RequiresUnreferencedCode (); } - static void CallTestRequiresDynamicCodeGuarded () + static void CallTestDynamicCodeGuarded () { if (RuntimeFeature.IsDynamicCodeSupported) RequiresDynamicCode (); } [ExpectedWarning ("IL3050", nameof (RequiresDynamicCode), ProducedBy = Tool.Analyzer | Tool.NativeAot)] - static void CallTestRequiresDynamicCodeUnguarded () + static void CallTestDynamicCodeUnguarded () { RequiresDynamicCode (); } @@ -519,8 +520,8 @@ public static void Test () { CallTestUnreferencedCodeGuarded (); CallTestUnreferencedCodeUnguarded (); - CallTestRequiresDynamicCodeGuarded (); - CallTestRequiresDynamicCodeUnguarded (); + CallTestDynamicCodeGuarded (); + CallTestDynamicCodeUnguarded (); CallTestAssemblyFilesGuarded (); CallTestAssemblyFilesUnguarded (); } @@ -1066,12 +1067,3 @@ class ClassWithRequires class RequiresAllGeneric<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] T> {} } } - -namespace ILLink.RoslynAnalyzer -{ - class TestFeatures - { - public static bool IsUnreferencedCodeSupported => true; - public static bool IsAssemblyFilesSupported => true; - } -} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml index c096cf07d6e7c1..db0bf370336765 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckDataFlowTestSubstitutions.xml @@ -1,5 +1,5 @@ - + diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs new file mode 100644 index 00000000000000..eae7212abba455 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs @@ -0,0 +1,441 @@ +// 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.Runtime.CompilerServices; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using ILLink.RoslynAnalyzer; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + // Note: the XML must be passed as an embedded resource named ILLink.Substitutions.xml, + // not as a separate substitution file, for it to work with NativeAot. + // Related: https://github.com/dotnet/runtime/issues/88647 + [SetupCompileBefore ("TestFeatures.dll", new[] { "Dependencies/TestFeatures.cs" }, + resources: new object[] { new [] { "FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml" } })] + // FeatureGuardAttribute is currently only supported by the analyzer. + // The same guard behavior is achieved for ILLink/ILCompiler using substitutions. + [SetupCompileResource ("FeatureGuardDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml")] + [IgnoreSubstitutions (false)] + public class FeatureGuardDataFlow + { + public static void Main () + { + DefineFeatureGuard.Test (); + GuardBodyValidation.Test (); + InvalidFeatureGuards.Test (); + } + + class DefineFeatureGuard { + [FeatureGuard(typeof(RequiresDynamicCodeAttribute))] + static bool GuardDynamicCode => RuntimeFeature.IsDynamicCodeSupported; + + static void TestGuardDynamicCode () + { + if (GuardDynamicCode) + RequiresDynamicCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool GuardUnreferencedCode => TestFeatures.IsUnreferencedCodeSupported; + + static void TestGuardUnreferencedCode () + { + if (GuardUnreferencedCode) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresAssemblyFilesAttribute))] + static bool GuardAssemblyFiles => TestFeatures.IsAssemblyFilesSupported; + + static void TestGuardAssemblyFiles () + { + if (GuardAssemblyFiles) + RequiresAssemblyFiles (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresDynamicCodeAttribute), ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresDynamicCodeAttribute))] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool GuardDynamicCodeAndUnreferencedCode => RuntimeFeature.IsDynamicCodeSupported && TestFeatures.IsUnreferencedCodeSupported; + + static void TestMultipleGuards () + { + if (GuardDynamicCodeAndUnreferencedCode) { + RequiresDynamicCode (); + RequiresUnreferencedCode (); + } + } + + public static void Test () + { + TestGuardDynamicCode (); + TestGuardUnreferencedCode (); + TestGuardAssemblyFiles (); + TestMultipleGuards (); + } + } + + class GuardBodyValidation { + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool ReturnTrueGuard => true; + + static void TestReturnTrueGuard () + { + if (ReturnTrueGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool ReturnFalseGuard => false; + + static void TestReturnFalseGuard () + { + if (ReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool OtherConditionGuard => OtherCondition (); + + static void TestOtherConditionGuard () + { + if (OtherConditionGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool DirectGuard => TestFeatures.IsUnreferencedCodeSupported; + + static void TestDirectGuard () + { + if (DirectGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool IndirectGuard => DirectGuard; + + static void TestIndirectGuard () + { + if (IndirectGuard) + RequiresUnreferencedCode (); + } + + // Analyzer doesn't understand this pattern because it compiles into a CFG that effectively + // looks like this: + // + // bool tmp; + // if (TestFeatures.IsUnreferencedCodeSupported) + // tmp = OtherCondition (); + // else + // tmp = false; + // return tmp; + // + // The analyzer doesn't do constant propagation of the boolean, so it doesn't know that + // the return value is always false when TestFeatures.IsUnreferencedCodeSupported is false. + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool AndGuard => TestFeatures.IsUnreferencedCodeSupported && OtherCondition (); + + static void TestAndGuard () + { + if (AndGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool OrGuard => TestFeatures.IsUnreferencedCodeSupported || OtherCondition (); + + static void TestOrGuard () + { + if (OrGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool NotGuard => !TestFeatures.IsUnreferencedCodeSupported; + + static void TestNotGuard () + { + if (NotGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool NotNotGuard => !!TestFeatures.IsUnreferencedCodeSupported; + + static void TestNotNotGuard () + { + if (NotNotGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool EqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported == true; + + static void TestEqualsTrueGuard () + { + if (EqualsTrueGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool EqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported == false; + + static void TestEqualsFalseGuard () + { + if (EqualsFalseGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool TrueEqualsGuard => true == TestFeatures.IsUnreferencedCodeSupported; + + static void TestTrueEqualsGuard () + { + if (TrueEqualsGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool FalseEqualsGuard => false == TestFeatures.IsUnreferencedCodeSupported; + + static void TestFalseEqualsGuard () + { + if (FalseEqualsGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool NotEqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported != true; + + static void TestNotEqualsTrueGuard () + { + if (NotEqualsTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool NotEqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported != false; + + static void TestNotEqualsFalseGuard () + { + if (NotEqualsFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool TrueNotEqualsGuard => true != TestFeatures.IsUnreferencedCodeSupported; + + static void TestTrueNotEqualsGuard () + { + if (TrueNotEqualsGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool FalseNotEqualsGuard => false != TestFeatures.IsUnreferencedCodeSupported; + + static void TestFalseNotEqualsGuard () + { + if (FalseNotEqualsGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool IsTrueGuard => TestFeatures.IsUnreferencedCodeSupported is true; + + static void TestIsTrueGuard () + { + if (IsTrueGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool IsNotTrueGuard => TestFeatures.IsUnreferencedCodeSupported is not true; + + static void TestIsNotTrueGuard () + { + if (IsNotTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool IsNotFalseGuard => TestFeatures.IsUnreferencedCodeSupported is not false; + + static void TestIsNotFalseGuard () + { + if (IsNotFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool IsFalseGuard => TestFeatures.IsUnreferencedCodeSupported is false; + + static void TestIsFalseGuard () + { + if (IsFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool IfGuard { + get { + if (TestFeatures.IsUnreferencedCodeSupported) + return true; + return false; + } + } + + static void TestIfGuard () + { + if (IfGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool ElseGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + return false; + else + return true; + } + } + + static void TestElseGuard () + { + if (ElseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool TernaryIfGuard => TestFeatures.IsUnreferencedCodeSupported ? true : false; + + static void TestTernaryIfGuard () + { + if (TernaryIfGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool TernaryElseGuard => !TestFeatures.IsUnreferencedCodeSupported ? false : true; + + static void TestTernaryElseGuard () + { + if (TernaryElseGuard) + RequiresUnreferencedCode (); + } + + public static void Test () + { + TestReturnTrueGuard (); + TestReturnFalseGuard (); + TestOtherConditionGuard (); + TestDirectGuard (); + TestIndirectGuard (); + TestAndGuard (); + TestOrGuard (); + TestNotGuard (); + TestNotNotGuard (); + TestEqualsTrueGuard (); + TestEqualsFalseGuard (); + TestTrueEqualsGuard (); + TestFalseEqualsGuard (); + TestNotEqualsTrueGuard (); + TestNotEqualsFalseGuard (); + TestTrueNotEqualsGuard (); + TestFalseNotEqualsGuard (); + TestIsTrueGuard (); + TestIsFalseGuard (); + TestIsNotTrueGuard (); + TestIsNotFalseGuard (); + TestIfGuard (); + TestElseGuard (); + TestTernaryIfGuard (); + TestTernaryElseGuard (); + } + } + + class InvalidFeatureGuards { + [ExpectedWarning ("IL4001", ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static int NonBooleanProperty => 0; + + [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] + static void TestNonBooleanProperty () + { + if (NonBooleanProperty == 0) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4001", ProducedBy = Tool.Analyzer)] + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + bool NonStaticProperty => true; + + [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] + static void TestNonStaticProperty () + { + var instance = new InvalidFeatureGuards (); + if (instance.NonStaticProperty) + RequiresUnreferencedCode (); + } + + // No warning for this case because we don't validate that the attribute usage matches + // the expected AttributeUsage.Property for assemblies that define their own version + // of FeatureGuardAttribute. + [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + static bool Method () => true; + + [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] + static void TestMethod () + { + if (Method ()) + RequiresUnreferencedCode (); + } + + public static void Test () + { + TestNonBooleanProperty (); + TestNonStaticProperty (); + TestMethod (); + } + } + + [RequiresDynamicCode (nameof (RequiresDynamicCode))] + static void RequiresDynamicCode () { } + + [RequiresUnreferencedCode (nameof (RequiresUnreferencedCode))] + static void RequiresUnreferencedCode () { } + + [RequiresAssemblyFiles (nameof (RequiresAssemblyFiles))] + static void RequiresAssemblyFiles () { } + + static bool OtherCondition () => true; + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml new file mode 100644 index 00000000000000..6ab85a04fd61b5 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 8246f2d9562ba4be4b678d00e0319cde43057685 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 16 Feb 2024 21:05:27 +0000 Subject: [PATCH 02/10] FeatureGuard -> FeatureCheck --- .../DataFlow/FeatureChecksVisitor.cs | 2 +- .../DataFlow/LocalDataFlowVisitor.cs | 2 +- .../DynamicallyAccessedMembersAnalyzer.cs | 6 +- .../ISymbolExtensions.cs | 8 +- .../RequiresAnalyzerBase.cs | 12 +-- .../TrimAnalysisReturnValuePattern.cs.cs | 20 ++--- .../TrimAnalysis/TrimAnalysisVisitor.cs | 10 +-- .../illink/src/ILLink.Shared/DiagnosticId.cs | 4 +- .../src/ILLink.Shared/SharedStrings.resx | 16 ++-- .../DataFlowTests.cs | 2 +- .../Support/FeatureCheckAttribute.cs | 17 ++++ .../Support/FeatureGuardAttribute.cs | 17 ---- ...ow.cs => FeatureCheckAttributeDataFlow.cs} | 87 ++++++++++--------- ...eckAttributeDataFlowTestSubstitutions.xml} | 8 +- 14 files changed, 107 insertions(+), 104 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs delete mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs rename src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/{FeatureGuardDataFlow.cs => FeatureCheckAttributeDataFlow.cs} (82%) rename src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/{FeatureGuardDataFlowTestSubstitutions.xml => FeatureCheckAttributeDataFlowTestSubstitutions.xml} (85%) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs index a8bf871072ddfd..11ae8168ffffba 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs @@ -50,7 +50,7 @@ public override FeatureChecksValue VisitPropertyReference (IPropertyReferenceOpe // A single property may serve as a feature check for multiple features. FeatureChecksValue featureChecks = FeatureChecksValue.None; foreach (var analyzer in _dataFlowAnalyzerContext.EnabledRequiresAnalyzers) { - if (analyzer.IsFeatureGuard (operation.Property, _dataFlowAnalyzerContext)) { + if (analyzer.IsFeatureCheck (operation.Property, _dataFlowAnalyzerContext)) { var featureCheck = new FeatureChecksValue (analyzer.RequiresAttributeFullyQualifiedName); featureChecks = featureChecks.And (featureCheck); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs index 84065ecc25fe8f..dc2345b5646323 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs @@ -119,7 +119,7 @@ public LocalDataFlowVisitor ( var current = state.Current; HandleReturnValue (branchValue, branchValueOperation, in current.Context); // Must be called for every return value even if it did not return an understood condition, - // because the non-understood conditions will produce warnings for FeatureGuard properties. + // because the non-understood conditions will produce warnings for FeatureCheck properties. HandleReturnConditionValue (conditionValue, branchValueOperation); return null; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index c7f7eb2eb6aa0b..c588577a082d82 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -21,7 +21,7 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute); public const string attributeArgument = "attributeArgument"; public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute; - public const string FullyQualifiedFeatureGuardAttribute = "System.Diagnostics.CodeAnalysis.FeatureGuardAttribute"; + public const string FullyQualifiedFeatureCheckAttribute = "System.Diagnostics.CodeAnalysis.FeatureCheckAttribute"; public static Lazy> RequiresAnalyzers { get; } = new Lazy> (GetRequiresAnalyzers); static ImmutableArray GetRequiresAnalyzers () => ImmutableArray.Create ( @@ -52,8 +52,8 @@ public static ImmutableArray GetSupportedDiagnostics () diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedTypeNameInTypeGetType)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedParameterInMethodCreateInstance)); diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ParametersOfAssemblyCreateInstanceCannotBeAnalyzed)); - diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ReturnValueDoesNotMatchFeatureGuards)); - diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.InvalidFeatureGuard)); + diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ReturnValueDoesNotMatchFeatureChecks)); + diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.InvalidFeatureCheck)); foreach (var requiresAnalyzer in RequiresAnalyzers.Value) { foreach (var diagnosticDescriptor in requiresAnalyzer.SupportedDiagnostics) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs index 03d6a8cbe5c138..0c39cd497ce8e1 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs @@ -71,20 +71,20 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes return (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers.ConstructorArguments[0].Value!; } - internal static ValueSet GetFeatureGuardAnnotations ( + internal static ValueSet GetFeatureCheckAnnotations ( this IPropertySymbol propertySymbol, IEnumerable enabledRequiresAnalyzers) { ImmutableArray.Builder featureSet = ImmutableArray.CreateBuilder (); foreach (var attributeData in propertySymbol.GetAttributes ()) { - if (IsFeatureGuardAttribute (attributeData, out string? featureName)) + if (IsFeatureCheckAttribute (attributeData, out string? featureName)) featureSet.Add (featureName); } return featureSet.Count == 0 ? ValueSet.Empty : new ValueSet (featureSet); - bool IsFeatureGuardAttribute (AttributeData attributeData, [NotNullWhen (true)] out string? featureName) { + bool IsFeatureCheckAttribute (AttributeData attributeData, [NotNullWhen (true)] out string? featureName) { featureName = null; - if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureGuardAttribute)) + if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureCheckAttribute)) return false; if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureType }]) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index 9b441b67060384..ffd5f57d348088 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -302,19 +302,19 @@ protected virtual bool CreateSpecialIncompatibleMembersDiagnostic ( // - custom feature checks defined in library code private protected virtual bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) => false; - internal static bool IsAnnotatedFeatureGuard (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext, string featureName) + internal static bool IsAnnotatedFeatureCheck (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext, string featureName) { - // Only respect FeatureGuardAttribute on static boolean properties. + // Only respect FeatureCheckAttribute on static boolean properties. if (!propertySymbol.IsStatic || propertySymbol.Type.SpecialType != SpecialType.System_Boolean) return false; - ValueSet featureGuards = propertySymbol.GetFeatureGuardAnnotations (dataFlowAnalyzerContext.EnabledRequiresAnalyzers); - return featureGuards.Contains (featureName); + ValueSet featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (dataFlowAnalyzerContext.EnabledRequiresAnalyzers); + return featureCheckAnnotations.Contains (featureName); } - internal bool IsFeatureGuard (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext) + internal bool IsFeatureCheck (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext) { - return IsAnnotatedFeatureGuard (propertySymbol, dataFlowAnalyzerContext, RequiresAttributeFullyQualifiedName) + return IsAnnotatedFeatureCheck (propertySymbol, dataFlowAnalyzerContext, RequiresAttributeFullyQualifiedName) || IsRequiresCheck (propertySymbol, dataFlowAnalyzerContext.Compilation); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs index 7839ea358e93db..f461e52fe12411 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs @@ -13,18 +13,18 @@ namespace ILLink.RoslynAnalyzer.TrimAnalysis public readonly record struct TrimAnalysisReturnValuePattern { public FeatureChecksValue ReturnValue { get; init; } - public ValueSet FeatureGuardAnnotations { get; init; } + public ValueSet FeatureCheckAnnotations { get; init; } public IOperation Operation { get; init; } public IPropertySymbol OwningSymbol { get; init; } public TrimAnalysisReturnValuePattern ( FeatureChecksValue returnValue, - ValueSet featureGuardAnnotations, + ValueSet featureCheckAnnotations, IOperation operation, IPropertySymbol owningSymbol) { ReturnValue = returnValue.DeepCopy (); - FeatureGuardAnnotations = featureGuardAnnotations.DeepCopy (); + FeatureCheckAnnotations = featureCheckAnnotations.DeepCopy (); Operation = operation; OwningSymbol = owningSymbol; } @@ -32,12 +32,12 @@ public TrimAnalysisReturnValuePattern ( public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) { var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); - // For now, feature guard validation is enabled only when trim analysis is enabled. + // For now, feature check validation is enabled only when trim analysis is enabled. if (context.EnableTrimAnalyzer) { if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean) { - // Warn about invalid feature guards (non-static or non-bool properties) + // Warn about invalid feature checks (non-static or non-bool properties) diagnosticContext.AddDiagnostic ( - DiagnosticId.InvalidFeatureGuard); + DiagnosticId.InvalidFeatureCheck); return diagnosticContext.Diagnostics; } @@ -45,12 +45,12 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte // For any feature that this property is declared to guard, // the abstract return value must include that feature // (indicating it is known to be enabled when the return value is true). - foreach (string featureGuard in FeatureGuardAnnotations.GetKnownValues ()) { - if (!returnValueFeatures.Contains (featureGuard)) { + foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { + if (!returnValueFeatures.Contains (feature)) { diagnosticContext.AddDiagnostic ( - DiagnosticId.ReturnValueDoesNotMatchFeatureGuards, + DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, OwningSymbol.GetDisplayName (), - featureGuard); + feature); } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index b20e99ef5f3fba..e11cb852c862b3 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -436,19 +436,19 @@ public override void HandleReturnConditionValue (FeatureChecksValue returnCondit if (OwningSymbol is not IMethodSymbol method) return; - // FeatureGuard validation needs to happen only for properties. + // FeatureCheck validation needs to happen only for properties. if (method.MethodKind != MethodKind.PropertyGet) return; IPropertySymbol propertySymbol = (IPropertySymbol) method.AssociatedSymbol!; - var featureGuards = propertySymbol.GetFeatureGuardAnnotations (_dataFlowAnalyzerContext.EnabledRequiresAnalyzers); + var featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (_dataFlowAnalyzerContext.EnabledRequiresAnalyzers); - // If there are no feature guards, there is nothing to validate. - if (featureGuards.IsEmpty()) + // If there are no feature checks, there is nothing to validate. + if (featureCheckAnnotations.IsEmpty()) return; TrimAnalysisPatterns.Add ( - new TrimAnalysisReturnValuePattern (returnConditionValue, featureGuards, operation, propertySymbol)); + new TrimAnalysisReturnValuePattern (returnConditionValue, featureCheckAnnotations, operation, propertySymbol)); } public override MultiValue HandleDelegateCreation (IMethodSymbol method, IOperation operation, in FeatureContext featureContext) diff --git a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs index 5d40efbbaaf994..1a3a27abbbdc27 100644 --- a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs +++ b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs @@ -204,8 +204,8 @@ public enum DiagnosticId RequiresDynamicCodeOnStaticConstructor = 3056, // Feature guard diagnostic ids. - ReturnValueDoesNotMatchFeatureGuards = 4000, - InvalidFeatureGuard = 4001 + ReturnValueDoesNotMatchFeatureChecks = 4000, + InvalidFeatureCheck = 4001 } public static class DiagnosticIdExtensions diff --git a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx index 16d387cfca0e77..18fab55a2fe394 100644 --- a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx +++ b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx @@ -1197,16 +1197,16 @@ Unused 'UnconditionalSuppressMessageAttribute' found. Consider removing the unused warning suppression. - - Return value does not match FeatureGuardAttribute '{1}'. + + Return value does not match FeatureCheckAttribute '{1}'. - - Return value does not match feature guards of the property. + + Return value does not match feature check annotations of the property. - - Invalid FeatureGuardAttribute. Ensure the attribute is plated on a static boolean property. + + Invalid FeatureCheckAttribute. Ensure the attribute is plated on a static boolean property. - - Invalid FeatureGuardAttribute. + + Invalid FeatureCheckAttribute. diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs index b934796d844e62..090a11b8616714 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DataFlowTests.cs @@ -150,7 +150,7 @@ public Task FeatureCheckDataFlow () } [Fact] - public Task FeatureGuardDataFlow () + public Task FeatureCheckAttributeDataFlow () { return RunTest (); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs new file mode 100644 index 00000000000000..2d284c2b3ea375 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureCheckAttribute.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + // Allow AttributeTargets.Method for testing invalid usages of a custom FeatureCheckAttribute + [AttributeUsage (AttributeTargets.Property | AttributeTargets.Method, Inherited=false)] + public sealed class FeatureCheckAttribute : Attribute + { + public Type FeatureType { get; } + + public FeatureCheckAttribute (Type featureType) + { + FeatureType = featureType; + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs deleted file mode 100644 index 3ef6c5205ed55d..00000000000000 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureGuardAttribute.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Diagnostics.CodeAnalysis -{ - // Allow AttributeTargets.Method for testing invalid usages of a custom FeatureGuardAttribute - [AttributeUsage(AttributeTargets.Property | AttributeTargets.Method, Inherited=false, AllowMultiple=true)] - public sealed class FeatureGuardAttribute : Attribute - { - public Type RequiresAttributeType { get; } - - public FeatureGuardAttribute(Type requiresAttributeType) - { - RequiresAttributeType = requiresAttributeType; - } - } -} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs similarity index 82% rename from src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs rename to src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs index eae7212abba455..e10a841c6f1c80 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs @@ -19,21 +19,21 @@ namespace Mono.Linker.Tests.Cases.DataFlow // Related: https://github.com/dotnet/runtime/issues/88647 [SetupCompileBefore ("TestFeatures.dll", new[] { "Dependencies/TestFeatures.cs" }, resources: new object[] { new [] { "FeatureCheckDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml" } })] - // FeatureGuardAttribute is currently only supported by the analyzer. + // FeatureCheckAttribute is currently only supported by the analyzer. // The same guard behavior is achieved for ILLink/ILCompiler using substitutions. - [SetupCompileResource ("FeatureGuardDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml")] + [SetupCompileResource ("FeatureCheckAttributeDataFlowTestSubstitutions.xml", "ILLink.Substitutions.xml")] [IgnoreSubstitutions (false)] - public class FeatureGuardDataFlow + public class FeatureCheckAttributeDataFlow { public static void Main () { - DefineFeatureGuard.Test (); + DefineFeatureCheck.Test (); GuardBodyValidation.Test (); - InvalidFeatureGuards.Test (); + InvalidFeatureChecks.Test (); } - class DefineFeatureGuard { - [FeatureGuard(typeof(RequiresDynamicCodeAttribute))] + class DefineFeatureCheck { + [FeatureCheck (typeof(RequiresDynamicCodeAttribute))] static bool GuardDynamicCode => RuntimeFeature.IsDynamicCodeSupported; static void TestGuardDynamicCode () @@ -42,7 +42,7 @@ static void TestGuardDynamicCode () RequiresDynamicCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool GuardUnreferencedCode => TestFeatures.IsUnreferencedCodeSupported; static void TestGuardUnreferencedCode () @@ -51,7 +51,7 @@ static void TestGuardUnreferencedCode () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresAssemblyFilesAttribute))] + [FeatureCheck (typeof(RequiresAssemblyFilesAttribute))] static bool GuardAssemblyFiles => TestFeatures.IsAssemblyFilesSupported; static void TestGuardAssemblyFiles () @@ -62,10 +62,13 @@ static void TestGuardAssemblyFiles () [ExpectedWarning ("IL4000", nameof (RequiresDynamicCodeAttribute), ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresDynamicCodeAttribute))] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(DynamicCodeAndUnreferencedCode))] static bool GuardDynamicCodeAndUnreferencedCode => RuntimeFeature.IsDynamicCodeSupported && TestFeatures.IsUnreferencedCodeSupported; + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + [FeatureDependsOn (typeof (RequiresUnreferencedCodeAttribute))] + static class DynamicCodeAndUnreferencedCode {} + static void TestMultipleGuards () { if (GuardDynamicCodeAndUnreferencedCode) { @@ -85,7 +88,7 @@ public static void Test () class GuardBodyValidation { [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool ReturnTrueGuard => true; static void TestReturnTrueGuard () @@ -95,7 +98,7 @@ static void TestReturnTrueGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool ReturnFalseGuard => false; static void TestReturnFalseGuard () @@ -105,7 +108,7 @@ static void TestReturnFalseGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool OtherConditionGuard => OtherCondition (); static void TestOtherConditionGuard () @@ -114,7 +117,7 @@ static void TestOtherConditionGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool DirectGuard => TestFeatures.IsUnreferencedCodeSupported; static void TestDirectGuard () @@ -123,7 +126,7 @@ static void TestDirectGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool IndirectGuard => DirectGuard; static void TestIndirectGuard () @@ -145,7 +148,7 @@ static void TestIndirectGuard () // The analyzer doesn't do constant propagation of the boolean, so it doesn't know that // the return value is always false when TestFeatures.IsUnreferencedCodeSupported is false. [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool AndGuard => TestFeatures.IsUnreferencedCodeSupported && OtherCondition (); static void TestAndGuard () @@ -155,7 +158,7 @@ static void TestAndGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool OrGuard => TestFeatures.IsUnreferencedCodeSupported || OtherCondition (); static void TestOrGuard () @@ -165,7 +168,7 @@ static void TestOrGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool NotGuard => !TestFeatures.IsUnreferencedCodeSupported; static void TestNotGuard () @@ -174,7 +177,7 @@ static void TestNotGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool NotNotGuard => !!TestFeatures.IsUnreferencedCodeSupported; static void TestNotNotGuard () @@ -183,7 +186,7 @@ static void TestNotNotGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool EqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported == true; static void TestEqualsTrueGuard () @@ -193,7 +196,7 @@ static void TestEqualsTrueGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool EqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported == false; static void TestEqualsFalseGuard () @@ -202,7 +205,7 @@ static void TestEqualsFalseGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool TrueEqualsGuard => true == TestFeatures.IsUnreferencedCodeSupported; static void TestTrueEqualsGuard () @@ -212,7 +215,7 @@ static void TestTrueEqualsGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool FalseEqualsGuard => false == TestFeatures.IsUnreferencedCodeSupported; static void TestFalseEqualsGuard () @@ -222,7 +225,7 @@ static void TestFalseEqualsGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool NotEqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported != true; static void TestNotEqualsTrueGuard () @@ -231,7 +234,7 @@ static void TestNotEqualsTrueGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool NotEqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported != false; static void TestNotEqualsFalseGuard () @@ -241,7 +244,7 @@ static void TestNotEqualsFalseGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool TrueNotEqualsGuard => true != TestFeatures.IsUnreferencedCodeSupported; static void TestTrueNotEqualsGuard () @@ -250,7 +253,7 @@ static void TestTrueNotEqualsGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool FalseNotEqualsGuard => false != TestFeatures.IsUnreferencedCodeSupported; static void TestFalseNotEqualsGuard () @@ -259,7 +262,7 @@ static void TestFalseNotEqualsGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool IsTrueGuard => TestFeatures.IsUnreferencedCodeSupported is true; static void TestIsTrueGuard () @@ -269,7 +272,7 @@ static void TestIsTrueGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool IsNotTrueGuard => TestFeatures.IsUnreferencedCodeSupported is not true; static void TestIsNotTrueGuard () @@ -278,7 +281,7 @@ static void TestIsNotTrueGuard () RequiresUnreferencedCode (); } - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool IsNotFalseGuard => TestFeatures.IsUnreferencedCodeSupported is not false; static void TestIsNotFalseGuard () @@ -288,7 +291,7 @@ static void TestIsNotFalseGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool IsFalseGuard => TestFeatures.IsUnreferencedCodeSupported is false; static void TestIsFalseGuard () @@ -299,7 +302,7 @@ static void TestIsFalseGuard () [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool IfGuard { get { if (TestFeatures.IsUnreferencedCodeSupported) @@ -316,7 +319,7 @@ static void TestIfGuard () [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool ElseGuard { get { if (!TestFeatures.IsUnreferencedCodeSupported) @@ -333,7 +336,7 @@ static void TestElseGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool TernaryIfGuard => TestFeatures.IsUnreferencedCodeSupported ? true : false; static void TestTernaryIfGuard () @@ -343,7 +346,7 @@ static void TestTernaryIfGuard () } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool TernaryElseGuard => !TestFeatures.IsUnreferencedCodeSupported ? false : true; static void TestTernaryElseGuard () @@ -382,9 +385,9 @@ public static void Test () } } - class InvalidFeatureGuards { + class InvalidFeatureChecks { [ExpectedWarning ("IL4001", ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static int NonBooleanProperty => 0; [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] @@ -395,21 +398,21 @@ static void TestNonBooleanProperty () } [ExpectedWarning ("IL4001", ProducedBy = Tool.Analyzer)] - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] bool NonStaticProperty => true; [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] static void TestNonStaticProperty () { - var instance = new InvalidFeatureGuards (); + var instance = new InvalidFeatureChecks (); if (instance.NonStaticProperty) RequiresUnreferencedCode (); } // No warning for this case because we don't validate that the attribute usage matches // the expected AttributeUsage.Property for assemblies that define their own version - // of FeatureGuardAttribute. - [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] + // of FeatureCheckAttributes. + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool Method () => true; [ExpectedWarning ("IL2026", nameof (RequiresUnreferencedCodeAttribute))] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml similarity index 85% rename from src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml rename to src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml index 6ab85a04fd61b5..f6ab92d9c19c94 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureGuardDataFlowTestSubstitutions.xml +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml @@ -1,17 +1,17 @@ - + - + - + - + From 5d21b79c59fe1b9086560ce637c85649545cb1e3 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 16 Feb 2024 22:30:21 +0000 Subject: [PATCH 03/10] Add support for FeatureDependsOn --- .../DynamicallyAccessedMembersAnalyzer.cs | 1 + .../ISymbolExtensions.cs | 48 ++++++++++++++----- .../Support/FeatureDependsOnAttribute.cs | 16 +++++++ 3 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index c588577a082d82..8c506606c292da 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -22,6 +22,7 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer public const string attributeArgument = "attributeArgument"; public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute; public const string FullyQualifiedFeatureCheckAttribute = "System.Diagnostics.CodeAnalysis.FeatureCheckAttribute"; + public const string FullyQualifiedFeatureDependsOnAttribute = "System.Diagnostics.CodeAnalysis.FeatureDependsOnAttribute"; public static Lazy> RequiresAnalyzers { get; } = new Lazy> (GetRequiresAnalyzers); static ImmutableArray GetRequiresAnalyzers () => ImmutableArray.Create ( diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs index 0c39cd497ce8e1..06999347aa0f9f 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs @@ -77,26 +77,50 @@ internal static ValueSet GetFeatureCheckAnnotations ( { ImmutableArray.Builder featureSet = ImmutableArray.CreateBuilder (); foreach (var attributeData in propertySymbol.GetAttributes ()) { - if (IsFeatureCheckAttribute (attributeData, out string? featureName)) - featureSet.Add (featureName); + if (IsFeatureCheckAttribute (attributeData, out INamedTypeSymbol? featureType)) + AddFeatures (featureType); } return featureSet.Count == 0 ? ValueSet.Empty : new ValueSet (featureSet); - bool IsFeatureCheckAttribute (AttributeData attributeData, [NotNullWhen (true)] out string? featureName) { - featureName = null; + void AddFeatures (INamedTypeSymbol featureType) { + foreach (var analyzer in enabledRequiresAnalyzers) { + if (featureType.HasName (analyzer.RequiresAttributeFullyQualifiedName)) { + featureSet.Add (analyzer.RequiresAttributeFullyQualifiedName); + // Don't need to continue looking for dependencies because the + // analyzer attributes don't have dependencies. + return; + } + } + + // Look at FeatureDependsOn attributes on the feature type. + foreach (var featureTypeAttributeData in featureType.GetAttributes ()) { + if (IsFeatureDependsOnAttribute (featureTypeAttributeData, out INamedTypeSymbol? featureDependsOnType)) + AddFeatures (featureDependsOnType); + } + + static bool IsFeatureDependsOnAttribute (AttributeData attributeData, [NotNullWhen (true)] out INamedTypeSymbol? featureType) { + featureType = null; + if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureDependsOnAttribute)) + return false; + + if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureTypeSymbol }]) + return false; + + featureType = featureTypeSymbol; + return true; + } + } + + static bool IsFeatureCheckAttribute (AttributeData attributeData, [NotNullWhen (true)] out INamedTypeSymbol? featureType) { + featureType = null; if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureCheckAttribute)) return false; - if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureType }]) + if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureTypeSymbol }]) return false; - foreach (var analyzer in enabledRequiresAnalyzers) { - if (featureType.HasName (analyzer.RequiresAttributeFullyQualifiedName)) { - featureName = analyzer.RequiresAttributeFullyQualifiedName; - return true; - } - } - return false; + featureType = featureTypeSymbol; + return true; } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs new file mode 100644 index 00000000000000..da9a19bbfebc6b --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Support/FeatureDependsOnAttribute.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + [AttributeUsage(AttributeTargets.Class, Inherited=false, AllowMultiple=true)] + public sealed class FeatureDependsOnAttribute : Attribute + { + public Type FeatureType { get; } + + public FeatureDependsOnAttribute(Type featureType) + { + FeatureType = featureType; + } + } +} From b244973ef425e3f407a8533927311a2bd6930438 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 27 Feb 2024 18:46:43 +0000 Subject: [PATCH 04/10] Add more tests, tweak guard validation --- .../DataFlow/FeatureChecksVisitor.cs | 2 +- .../ISymbolExtensions.cs | 16 ++---- .../RequiresAnalyzerBase.cs | 10 ++-- .../TrimAnalysisReturnValuePattern.cs.cs | 17 ++++--- .../TrimAnalysis/TrimAnalysisVisitor.cs | 2 +- .../DataFlow/FeatureCheckAttributeDataFlow.cs | 51 +++++++++++++++++++ 6 files changed, 74 insertions(+), 24 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs index 11ae8168ffffba..905e8e1fb91e61 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs @@ -50,7 +50,7 @@ public override FeatureChecksValue VisitPropertyReference (IPropertyReferenceOpe // A single property may serve as a feature check for multiple features. FeatureChecksValue featureChecks = FeatureChecksValue.None; foreach (var analyzer in _dataFlowAnalyzerContext.EnabledRequiresAnalyzers) { - if (analyzer.IsFeatureCheck (operation.Property, _dataFlowAnalyzerContext)) { + if (analyzer.IsFeatureCheck (operation.Property, _dataFlowAnalyzerContext.Compilation)) { var featureCheck = new FeatureChecksValue (analyzer.RequiresAttributeFullyQualifiedName); featureChecks = featureChecks.And (featureCheck); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs index 06999347aa0f9f..b19bbb185e3ac8 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs @@ -71,9 +71,7 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes return (DynamicallyAccessedMemberTypes) dynamicallyAccessedMembers.ConstructorArguments[0].Value!; } - internal static ValueSet GetFeatureCheckAnnotations ( - this IPropertySymbol propertySymbol, - IEnumerable enabledRequiresAnalyzers) + internal static ValueSet GetFeatureCheckAnnotations (this IPropertySymbol propertySymbol) { ImmutableArray.Builder featureSet = ImmutableArray.CreateBuilder (); foreach (var attributeData in propertySymbol.GetAttributes ()) { @@ -83,14 +81,10 @@ internal static ValueSet GetFeatureCheckAnnotations ( return featureSet.Count == 0 ? ValueSet.Empty : new ValueSet (featureSet); void AddFeatures (INamedTypeSymbol featureType) { - foreach (var analyzer in enabledRequiresAnalyzers) { - if (featureType.HasName (analyzer.RequiresAttributeFullyQualifiedName)) { - featureSet.Add (analyzer.RequiresAttributeFullyQualifiedName); - // Don't need to continue looking for dependencies because the - // analyzer attributes don't have dependencies. - return; - } - } + var featureName = featureType.GetDisplayName (); + if (featureSet.Contains (featureName)) + return; + featureSet.Add (featureName); // Look at FeatureDependsOn attributes on the feature type. foreach (var featureTypeAttributeData in featureType.GetAttributes ()) { diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index ffd5f57d348088..5b4c820c4c6d55 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -302,20 +302,20 @@ protected virtual bool CreateSpecialIncompatibleMembersDiagnostic ( // - custom feature checks defined in library code private protected virtual bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) => false; - internal static bool IsAnnotatedFeatureCheck (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext, string featureName) + internal static bool IsAnnotatedFeatureCheck (IPropertySymbol propertySymbol, string featureName) { // Only respect FeatureCheckAttribute on static boolean properties. if (!propertySymbol.IsStatic || propertySymbol.Type.SpecialType != SpecialType.System_Boolean) return false; - ValueSet featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (dataFlowAnalyzerContext.EnabledRequiresAnalyzers); + ValueSet featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (); return featureCheckAnnotations.Contains (featureName); } - internal bool IsFeatureCheck (IPropertySymbol propertySymbol, DataFlowAnalyzerContext dataFlowAnalyzerContext) + internal bool IsFeatureCheck (IPropertySymbol propertySymbol, Compilation compilation) { - return IsAnnotatedFeatureCheck (propertySymbol, dataFlowAnalyzerContext, RequiresAttributeFullyQualifiedName) - || IsRequiresCheck (propertySymbol, dataFlowAnalyzerContext.Compilation); + return IsAnnotatedFeatureCheck (propertySymbol, RequiresAttributeFullyQualifiedName) + || IsRequiresCheck (propertySymbol, compilation); } internal bool CheckAndCreateRequiresDiagnostic ( diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs index f461e52fe12411..6800ab1b10464c 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs @@ -42,15 +42,20 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte } ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; - // For any feature that this property is declared to guard, + // For any analyzer-supported feature that this property is declared to guard, // the abstract return value must include that feature // (indicating it is known to be enabled when the return value is true). foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { - if (!returnValueFeatures.Contains (feature)) { - diagnosticContext.AddDiagnostic ( - DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, - OwningSymbol.GetDisplayName (), - feature); + foreach (var analyzer in context.EnabledRequiresAnalyzers) { + if (feature != analyzer.RequiresAttributeFullyQualifiedName) + continue; + + if (!returnValueFeatures.Contains (feature)) { + diagnosticContext.AddDiagnostic ( + DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, + OwningSymbol.GetDisplayName (), + feature); + } } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index e11cb852c862b3..7d324187d1b817 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -441,7 +441,7 @@ public override void HandleReturnConditionValue (FeatureChecksValue returnCondit return; IPropertySymbol propertySymbol = (IPropertySymbol) method.AssociatedSymbol!; - var featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (_dataFlowAnalyzerContext.EnabledRequiresAnalyzers); + var featureCheckAnnotations = propertySymbol.GetFeatureCheckAnnotations (); // If there are no feature checks, there is nothing to validate. if (featureCheckAnnotations.IsEmpty()) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs index e10a841c6f1c80..399aac50460111 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs @@ -77,12 +77,63 @@ static void TestMultipleGuards () } } + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + static class DynamicCode1 {} + + [FeatureDependsOn (typeof (DynamicCode1))] + static class DynamicCode2 {} + + [FeatureCheck (typeof (DynamicCode2))] + static bool GuardDynamicCodeIndirect => RuntimeFeature.IsDynamicCodeSupported; + + static void TestIndirectGuards () + { + if (GuardDynamicCodeIndirect) + RequiresDynamicCode (); + } + + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + [FeatureDependsOn (typeof (DynamicCodeCycle))] + static class DynamicCodeCycle {} + + [FeatureCheck (typeof (DynamicCodeCycle))] + static bool GuardDynamicCodeCycle => RuntimeFeature.IsDynamicCodeSupported; + + [FeatureCheck (typeof (DynamicCodeCycle))] + static void TestFeatureDependencyCycle1 () + { + if (GuardDynamicCodeCycle) + RequiresDynamicCode (); + } + + [FeatureDependsOn (typeof (DynamicCodeCycle2_B))] + static class DynamicCodeCycle2_A {} + + [FeatureDependsOn (typeof (RequiresDynamicCodeAttribute))] + [FeatureDependsOn (typeof (DynamicCodeCycle2_A))] + static class DynamicCodeCycle2_B {} + + [FeatureDependsOn (typeof (DynamicCodeCycle2_A))] + static class DynamicCodeCycle2 {} + + [FeatureCheck (typeof (DynamicCodeCycle2))] + static bool GuardDynamicCodeCycle2 => RuntimeFeature.IsDynamicCodeSupported; + + static void TestFeatureDependencyCycle2 () + { + if (GuardDynamicCodeCycle2) + RequiresDynamicCode (); + } + public static void Test () { TestGuardDynamicCode (); TestGuardUnreferencedCode (); TestGuardAssemblyFiles (); TestMultipleGuards (); + TestIndirectGuards (); + TestFeatureDependencyCycle1 (); + TestFeatureDependencyCycle2 (); } } From 4cf6681b483bcb0bad3cd34ca3766dd4c646b5d9 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 27 Feb 2024 19:17:14 +0000 Subject: [PATCH 05/10] Support guards that return false --- .../ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs | 2 ++ .../DataFlow/FeatureChecksVisitor.cs | 9 +++++++++ .../DataFlow/FeatureCheckAttributeDataFlow.cs | 1 - 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs index 086e16bddfcd37..cfcddd751d60a9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs @@ -18,6 +18,8 @@ public record struct FeatureChecksValue : INegate, IDeepCopy public ValueSet EnabledFeatures; public ValueSet DisabledFeatures; + public static readonly FeatureChecksValue All = new FeatureChecksValue (ValueSet.Unknown, ValueSet.Empty); + public static FeatureChecksValue None = new FeatureChecksValue (ValueSet.Empty, ValueSet.Empty); public FeatureChecksValue (string enabledFeature) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs index 905e8e1fb91e61..7c0935eb05ea48 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksVisitor.cs @@ -68,6 +68,15 @@ public override FeatureChecksValue VisitUnaryOperator (IUnaryOperation operation return context.Negate (); } + public override FeatureChecksValue VisitLiteral (ILiteralOperation operation, StateValue state) + { + // 'false' can guard any feature + if (GetConstantBool (operation.ConstantValue) is false) + return FeatureChecksValue.All; + + return FeatureChecksValue.None; + } + public bool? GetLiteralBool (IOperation operation) { if (operation is not ILiteralOperation literal) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs index 399aac50460111..0da66db27f36cf 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs @@ -148,7 +148,6 @@ static void TestReturnTrueGuard () RequiresUnreferencedCode (); } - [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool ReturnFalseGuard => false; From 5c6c187462a7b5e48fc46c5a948316b113e50e08 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 27 Feb 2024 19:28:29 +0000 Subject: [PATCH 06/10] Clean up --- .../DataFlow/FeatureCheckAttributeDataFlow.cs | 4 ++-- .../FeatureCheckAttributeDataFlowTestSubstitutions.xml | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs index 0da66db27f36cf..7efad8899573d8 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs @@ -86,7 +86,7 @@ static class DynamicCode2 {} [FeatureCheck (typeof (DynamicCode2))] static bool GuardDynamicCodeIndirect => RuntimeFeature.IsDynamicCodeSupported; - static void TestIndirectGuards () + static void TestIndirectGuard () { if (GuardDynamicCodeIndirect) RequiresDynamicCode (); @@ -131,7 +131,7 @@ public static void Test () TestGuardUnreferencedCode (); TestGuardAssemblyFiles (); TestMultipleGuards (); - TestIndirectGuards (); + TestIndirectGuard (); TestFeatureDependencyCycle1 (); TestFeatureDependencyCycle2 (); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml index f6ab92d9c19c94..bc1a3f4ac46569 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml @@ -2,13 +2,13 @@ - + + + - - From b78981f9708225e3432213f09e0fb0296f6d994e Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 28 Feb 2024 00:44:46 +0000 Subject: [PATCH 07/10] Add missing check, split up guard body tests --- .../TrimAnalysisReturnValuePattern.cs.cs | 28 +- .../DataFlow/FeatureCheckAttributeDataFlow.cs | 335 ++++++++++++------ ...heckAttributeDataFlowTestSubstitutions.xml | 38 +- 3 files changed, 267 insertions(+), 134 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs index 6800ab1b10464c..7bd30c31a85a18 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs @@ -41,20 +41,22 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte return diagnosticContext.Diagnostics; } - ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; - // For any analyzer-supported feature that this property is declared to guard, - // the abstract return value must include that feature - // (indicating it is known to be enabled when the return value is true). - foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { - foreach (var analyzer in context.EnabledRequiresAnalyzers) { - if (feature != analyzer.RequiresAttributeFullyQualifiedName) - continue; + if (ReturnValue != FeatureChecksValue.All) { + ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; + // For any analyzer-supported feature that this property is declared to guard, + // the abstract return value must include that feature + // (indicating it is known to be enabled when the return value is true). + foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { + foreach (var analyzer in context.EnabledRequiresAnalyzers) { + if (feature != analyzer.RequiresAttributeFullyQualifiedName) + continue; - if (!returnValueFeatures.Contains (feature)) { - diagnosticContext.AddDiagnostic ( - DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, - OwningSymbol.GetDisplayName (), - feature); + if (!returnValueFeatures.Contains (feature)) { + diagnosticContext.AddDiagnostic ( + DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, + OwningSymbol.GetDisplayName (), + feature); + } } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs index 7efad8899573d8..d8aa258c377546 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlow.cs @@ -28,7 +28,8 @@ public class FeatureCheckAttributeDataFlow public static void Main () { DefineFeatureCheck.Test (); - GuardBodyValidation.Test (); + ValidGuardBodies.Test (); + InvalidGuardBodies.Test (); InvalidFeatureChecks.Test (); } @@ -137,16 +138,7 @@ public static void Test () } } - class GuardBodyValidation { - [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool ReturnTrueGuard => true; - - static void TestReturnTrueGuard () - { - if (ReturnTrueGuard) - RequiresUnreferencedCode (); - } + class ValidGuardBodies { [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool ReturnFalseGuard => false; @@ -157,16 +149,6 @@ static void TestReturnFalseGuard () RequiresUnreferencedCode (); } - [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool OtherConditionGuard => OtherCondition (); - - static void TestOtherConditionGuard () - { - if (OtherConditionGuard) - RequiresUnreferencedCode (); - } - [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] static bool DirectGuard => TestFeatures.IsUnreferencedCodeSupported; @@ -207,231 +189,370 @@ static void TestAndGuard () RequiresUnreferencedCode (); } - [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool OrGuard => TestFeatures.IsUnreferencedCodeSupported || OtherCondition (); + static bool NotNotGuard => !!TestFeatures.IsUnreferencedCodeSupported; - static void TestOrGuard () + static void TestNotNotGuard () { - if (OrGuard) + if (NotNotGuard) RequiresUnreferencedCode (); } - [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool NotGuard => !TestFeatures.IsUnreferencedCodeSupported; + static bool EqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported == true; - static void TestNotGuard () + static void TestEqualsTrueGuard () { - if (NotGuard) + if (EqualsTrueGuard) RequiresUnreferencedCode (); } [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool NotNotGuard => !!TestFeatures.IsUnreferencedCodeSupported; + static bool TrueEqualsGuard => true == TestFeatures.IsUnreferencedCodeSupported; - static void TestNotNotGuard () + static void TestTrueEqualsGuard () { - if (NotNotGuard) + if (TrueEqualsGuard) RequiresUnreferencedCode (); } [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool EqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported == true; + static bool NotEqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported != false; - static void TestEqualsTrueGuard () + static void TestNotEqualsFalseGuard () { - if (EqualsTrueGuard) + if (NotEqualsFalseGuard) RequiresUnreferencedCode (); } - [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool EqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported == false; + static bool FalseNotEqualsGuard => false != TestFeatures.IsUnreferencedCodeSupported; - static void TestEqualsFalseGuard () + static void TestFalseNotEqualsGuard () { - if (EqualsFalseGuard) + if (FalseNotEqualsGuard) RequiresUnreferencedCode (); } [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool TrueEqualsGuard => true == TestFeatures.IsUnreferencedCodeSupported; + static bool IsTrueGuard => TestFeatures.IsUnreferencedCodeSupported is true; - static void TestTrueEqualsGuard () + static void TestIsTrueGuard () { - if (TrueEqualsGuard) + if (IsTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsNotFalseGuard => TestFeatures.IsUnreferencedCodeSupported is not false; + + static void TestIsNotFalseGuard () + { + if (IsNotFalseGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool FalseEqualsGuard => false == TestFeatures.IsUnreferencedCodeSupported; + static bool IfReturnTrueGuard { + get { + if (TestFeatures.IsUnreferencedCodeSupported) + return true; + return false; + } + } - static void TestFalseEqualsGuard () + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool ElseReturnTrueGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + return false; + else + return true; + } + } + + static void TestElseReturnTrueGuard () { - if (FalseEqualsGuard) + if (ElseReturnTrueGuard) + RequiresUnreferencedCode (); + } + + static void TestIfReturnTrueGuard () + { + if (IfReturnTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertReturnFalseGuard { + get { + Debug.Assert (TestFeatures.IsUnreferencedCodeSupported); + return false; + } + } + + static void TestAssertReturnFalseGuard () + { + if (AssertReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertNotReturnFalseGuard { + get { + Debug.Assert (!TestFeatures.IsUnreferencedCodeSupported); + return false; + } + } + + static void TestAssertNotReturnFalseGuard () + { + if (AssertNotReturnFalseGuard) + RequiresUnreferencedCode (); + } + + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertReturnTrueGuard { + get { + Debug.Assert (TestFeatures.IsUnreferencedCodeSupported); + return true; + } + } + + static void TestAssertReturnTrueGuard () + { + if (AssertReturnTrueGuard) + RequiresUnreferencedCode (); + } + + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool ThrowGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + throw new Exception (); + return false; + } + } + + static void TestThrowGuard () + { + if (ThrowGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool NotEqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported != true; + static bool TernaryIfGuard => TestFeatures.IsUnreferencedCodeSupported ? true : false; - static void TestNotEqualsTrueGuard () + static void TestTernaryIfGuard () { - if (NotEqualsTrueGuard) + if (TernaryIfGuard) RequiresUnreferencedCode (); } + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool NotEqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported != false; + static bool TernaryElseGuard => !TestFeatures.IsUnreferencedCodeSupported ? false : true; - static void TestNotEqualsFalseGuard () + static void TestTernaryElseGuard () { - if (NotEqualsFalseGuard) + if (TernaryElseGuard) RequiresUnreferencedCode (); } + public static void Test () + { + TestDirectGuard (); + TestIndirectGuard (); + + TestReturnFalseGuard (); + TestAndGuard (); + TestNotNotGuard (); + TestEqualsTrueGuard (); + TestTrueEqualsGuard (); + TestNotEqualsFalseGuard (); + TestFalseNotEqualsGuard (); + TestIsTrueGuard (); + TestIsNotFalseGuard (); + TestIfReturnTrueGuard (); + TestElseReturnTrueGuard (); + TestAssertReturnFalseGuard (); + TestAssertNotReturnFalseGuard (); + TestAssertReturnTrueGuard (); + TestThrowGuard (); + TestTernaryIfGuard (); + TestTernaryElseGuard (); + } + } + + class InvalidGuardBodies { [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool TrueNotEqualsGuard => true != TestFeatures.IsUnreferencedCodeSupported; + static bool ReturnTrueGuard => true; - static void TestTrueNotEqualsGuard () + static void TestReturnTrueGuard () { - if (TrueNotEqualsGuard) + if (ReturnTrueGuard) RequiresUnreferencedCode (); } + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool FalseNotEqualsGuard => false != TestFeatures.IsUnreferencedCodeSupported; + static bool OtherConditionGuard => OtherCondition (); - static void TestFalseNotEqualsGuard () + static void TestOtherConditionGuard () { - if (FalseNotEqualsGuard) + if (OtherConditionGuard) RequiresUnreferencedCode (); } + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool IsTrueGuard => TestFeatures.IsUnreferencedCodeSupported is true; + static bool OrGuard => TestFeatures.IsUnreferencedCodeSupported || OtherCondition (); - static void TestIsTrueGuard () + static void TestOrGuard () { - if (IsTrueGuard) + if (OrGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool IsNotTrueGuard => TestFeatures.IsUnreferencedCodeSupported is not true; + static bool NotGuard => !TestFeatures.IsUnreferencedCodeSupported; - static void TestIsNotTrueGuard () + static void TestNotGuard () { - if (IsNotTrueGuard) + if (NotGuard) RequiresUnreferencedCode (); } + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool IsNotFalseGuard => TestFeatures.IsUnreferencedCodeSupported is not false; + static bool EqualsFalseGuard => TestFeatures.IsUnreferencedCodeSupported == false; - static void TestIsNotFalseGuard () + static void TestEqualsFalseGuard () { - if (IsNotFalseGuard) + if (EqualsFalseGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool IsFalseGuard => TestFeatures.IsUnreferencedCodeSupported is false; + static bool FalseEqualsGuard => false == TestFeatures.IsUnreferencedCodeSupported; - static void TestIsFalseGuard () + static void TestFalseEqualsGuard () { - if (IsFalseGuard) + if (FalseEqualsGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool NotEqualsTrueGuard => TestFeatures.IsUnreferencedCodeSupported != true; + + static void TestNotEqualsTrueGuard () + { + if (NotEqualsTrueGuard) + RequiresUnreferencedCode (); + } + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool IfGuard { - get { - if (TestFeatures.IsUnreferencedCodeSupported) - return true; - return false; - } + static bool TrueNotEqualsGuard => true != TestFeatures.IsUnreferencedCodeSupported; + + static void TestTrueNotEqualsGuard () + { + if (TrueNotEqualsGuard) + RequiresUnreferencedCode (); } - static void TestIfGuard () + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsNotTrueGuard => TestFeatures.IsUnreferencedCodeSupported is not true; + + static void TestIsNotTrueGuard () { - if (IfGuard) + if (IsNotTrueGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] + [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] + static bool IsFalseGuard => TestFeatures.IsUnreferencedCodeSupported is false; + + static void TestIsFalseGuard () + { + if (IsFalseGuard) + RequiresUnreferencedCode (); + } + [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool ElseGuard { + static bool IfReturnFalseGuard { get { - if (!TestFeatures.IsUnreferencedCodeSupported) + if (TestFeatures.IsUnreferencedCodeSupported) return false; - else - return true; + return true; } } - static void TestElseGuard () + static void TestIfReturnFalseGuard () { - if (ElseGuard) + if (IfReturnFalseGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool TernaryIfGuard => TestFeatures.IsUnreferencedCodeSupported ? true : false; + static bool ElseReturnFalseGuard { + get { + if (!TestFeatures.IsUnreferencedCodeSupported) + return true; + else + return false; + } + } - static void TestTernaryIfGuard () + static void TestElseReturnFalseGuard () { - if (TernaryIfGuard) + if (ElseReturnFalseGuard) RequiresUnreferencedCode (); } [ExpectedWarning ("IL4000", nameof (RequiresUnreferencedCodeAttribute), ProducedBy = Tool.Analyzer)] - [FeatureCheck (typeof(RequiresUnreferencedCodeAttribute))] - static bool TernaryElseGuard => !TestFeatures.IsUnreferencedCodeSupported ? false : true; + [FeatureCheck (typeof (RequiresUnreferencedCodeAttribute))] + static bool AssertNotReturnTrueGuard { + get { + Debug.Assert (!TestFeatures.IsUnreferencedCodeSupported); + return true; + } + } - static void TestTernaryElseGuard () + static void TestAssertNotReturnTrueGuard () { - if (TernaryElseGuard) + if (AssertNotReturnTrueGuard) RequiresUnreferencedCode (); } public static void Test () { - TestReturnTrueGuard (); - TestReturnFalseGuard (); TestOtherConditionGuard (); - TestDirectGuard (); - TestIndirectGuard (); - TestAndGuard (); + + TestReturnTrueGuard (); TestOrGuard (); TestNotGuard (); - TestNotNotGuard (); - TestEqualsTrueGuard (); TestEqualsFalseGuard (); - TestTrueEqualsGuard (); TestFalseEqualsGuard (); TestNotEqualsTrueGuard (); - TestNotEqualsFalseGuard (); TestTrueNotEqualsGuard (); - TestFalseNotEqualsGuard (); - TestIsTrueGuard (); - TestIsFalseGuard (); TestIsNotTrueGuard (); - TestIsNotFalseGuard (); - TestIfGuard (); - TestElseGuard (); - TestTernaryIfGuard (); - TestTernaryElseGuard (); + TestIsFalseGuard (); + TestIfReturnFalseGuard (); + TestElseReturnFalseGuard (); + TestAssertNotReturnTrueGuard (); } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml index bc1a3f4ac46569..828ef795ae3721 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/FeatureCheckAttributeDataFlowTestSubstitutions.xml @@ -11,32 +11,42 @@ - - - - + + - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + From 45337ce56e0516238147842091206c119d181fff Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 28 Feb 2024 15:29:32 -0800 Subject: [PATCH 08/10] Update src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs Co-authored-by: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> --- .../src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs index cfcddd751d60a9..028628f2dd5934 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChecksValue.cs @@ -20,7 +20,7 @@ public record struct FeatureChecksValue : INegate, IDeepCopy public static readonly FeatureChecksValue All = new FeatureChecksValue (ValueSet.Unknown, ValueSet.Empty); - public static FeatureChecksValue None = new FeatureChecksValue (ValueSet.Empty, ValueSet.Empty); + public static readonly FeatureChecksValue None = new FeatureChecksValue (ValueSet.Empty, ValueSet.Empty); public FeatureChecksValue (string enabledFeature) { From 57f5e4eddb5fc97ad3f16e9bb8bc923289e5da9a Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 28 Feb 2024 15:29:58 -0800 Subject: [PATCH 09/10] Update src/tools/illink/src/ILLink.Shared/SharedStrings.resx Co-authored-by: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> --- src/tools/illink/src/ILLink.Shared/SharedStrings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx index 18fab55a2fe394..ae3d0f63b27afc 100644 --- a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx +++ b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx @@ -1204,7 +1204,7 @@ Return value does not match feature check annotations of the property. - Invalid FeatureCheckAttribute. Ensure the attribute is plated on a static boolean property. + Invalid FeatureCheckAttribute. The attribute must be placed on a static boolean property with only a 'get' accessor. Invalid FeatureCheckAttribute. From c9c244bff066c946bf516db72ec8aca0f50881c1 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 28 Feb 2024 23:59:52 +0000 Subject: [PATCH 10/10] PR feedback - Add more detail to feature check validation message - Use existing attribute helper to get AttributeData by attribute name - Replace unnecessary ImmutableArray Builder - Invert some if conditions to return early - Rename TrimAnalysisReturnValuePattern -> FeatureCheckReturnValuePattern --- .../ISymbolExtensions.cs | 39 ++--------- .../FeatureCheckReturnValuePattern.cs | 70 +++++++++++++++++++ .../TrimAnalysis/TrimAnalysisPatternStore.cs | 12 ++-- .../TrimAnalysisReturnValuePattern.cs.cs | 68 ------------------ .../TrimAnalysis/TrimAnalysisVisitor.cs | 2 +- .../src/ILLink.Shared/SharedStrings.resx | 2 +- 6 files changed, 85 insertions(+), 108 deletions(-) create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs delete mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs index b19bbb185e3ac8..42ad2f9c9ae177 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs @@ -73,48 +73,23 @@ internal static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypes internal static ValueSet GetFeatureCheckAnnotations (this IPropertySymbol propertySymbol) { - ImmutableArray.Builder featureSet = ImmutableArray.CreateBuilder (); - foreach (var attributeData in propertySymbol.GetAttributes ()) { - if (IsFeatureCheckAttribute (attributeData, out INamedTypeSymbol? featureType)) + HashSet featureSet = new (); + foreach (var attributeData in propertySymbol.GetAttributes (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureCheckAttribute)) { + if (attributeData.ConstructorArguments is [TypedConstant { Value: INamedTypeSymbol featureType }]) AddFeatures (featureType); } return featureSet.Count == 0 ? ValueSet.Empty : new ValueSet (featureSet); void AddFeatures (INamedTypeSymbol featureType) { var featureName = featureType.GetDisplayName (); - if (featureSet.Contains (featureName)) + if (!featureSet.Add (featureName)) return; - featureSet.Add (featureName); // Look at FeatureDependsOn attributes on the feature type. - foreach (var featureTypeAttributeData in featureType.GetAttributes ()) { - if (IsFeatureDependsOnAttribute (featureTypeAttributeData, out INamedTypeSymbol? featureDependsOnType)) - AddFeatures (featureDependsOnType); + foreach (var featureTypeAttributeData in featureType.GetAttributes (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureDependsOnAttribute)) { + if (featureTypeAttributeData.ConstructorArguments is [TypedConstant { Value: INamedTypeSymbol featureTypeSymbol }]) + AddFeatures (featureTypeSymbol); } - - static bool IsFeatureDependsOnAttribute (AttributeData attributeData, [NotNullWhen (true)] out INamedTypeSymbol? featureType) { - featureType = null; - if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureDependsOnAttribute)) - return false; - - if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureTypeSymbol }]) - return false; - - featureType = featureTypeSymbol; - return true; - } - } - - static bool IsFeatureCheckAttribute (AttributeData attributeData, [NotNullWhen (true)] out INamedTypeSymbol? featureType) { - featureType = null; - if (attributeData.AttributeClass is not { } attrClass || !attrClass.HasName (DynamicallyAccessedMembersAnalyzer.FullyQualifiedFeatureCheckAttribute)) - return false; - - if (attributeData.ConstructorArguments is not [TypedConstant { Value: INamedTypeSymbol featureTypeSymbol }]) - return false; - - featureType = featureTypeSymbol; - return true; } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs new file mode 100644 index 00000000000000..b46ef52e1f3a62 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs @@ -0,0 +1,70 @@ +// 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 ILLink.Shared; +using ILLink.Shared.DataFlow; +using ILLink.Shared.TrimAnalysis; +using ILLink.RoslynAnalyzer.DataFlow; +using Microsoft.CodeAnalysis; + +namespace ILLink.RoslynAnalyzer.TrimAnalysis +{ + public readonly record struct FeatureCheckReturnValuePattern + { + public FeatureChecksValue ReturnValue { get; init; } + public ValueSet FeatureCheckAnnotations { get; init; } + public IOperation Operation { get; init; } + public IPropertySymbol OwningSymbol { get; init; } + + public FeatureCheckReturnValuePattern ( + FeatureChecksValue returnValue, + ValueSet featureCheckAnnotations, + IOperation operation, + IPropertySymbol owningSymbol) + { + ReturnValue = returnValue.DeepCopy (); + FeatureCheckAnnotations = featureCheckAnnotations.DeepCopy (); + Operation = operation; + OwningSymbol = owningSymbol; + } + + public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) + { + var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); + // For now, feature check validation is enabled only when trim analysis is enabled. + if (!context.EnableTrimAnalyzer) + return diagnosticContext.Diagnostics; + + if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean) { + // Warn about invalid feature checks (non-static or non-bool properties) + diagnosticContext.AddDiagnostic ( + DiagnosticId.InvalidFeatureCheck); + return diagnosticContext.Diagnostics; + } + + if (ReturnValue == FeatureChecksValue.All) + return diagnosticContext.Diagnostics; + + ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; + // For any analyzer-supported feature that this property is declared to guard, + // the abstract return value must include that feature + // (indicating it is known to be enabled when the return value is true). + foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { + foreach (var analyzer in context.EnabledRequiresAnalyzers) { + if (feature != analyzer.RequiresAttributeFullyQualifiedName) + continue; + + if (!returnValueFeatures.Contains (feature)) { + diagnosticContext.AddDiagnostic ( + DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, + OwningSymbol.GetDisplayName (), + feature); + } + } + } + + return diagnosticContext.Diagnostics; + } + } +} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs index bea79901aaf0fa..dd66d802934be6 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisPatternStore.cs @@ -17,7 +17,7 @@ public readonly struct TrimAnalysisPatternStore readonly Dictionary GenericInstantiationPatterns; readonly Dictionary MethodCallPatterns; readonly Dictionary ReflectionAccessPatterns; - readonly Dictionary ReturnValuePatterns; + readonly Dictionary FeatureCheckReturnValuePatterns; readonly ValueSetLattice Lattice; readonly FeatureContextLattice FeatureContextLattice; @@ -30,7 +30,7 @@ public TrimAnalysisPatternStore ( GenericInstantiationPatterns = new Dictionary (); MethodCallPatterns = new Dictionary (); ReflectionAccessPatterns = new Dictionary (); - ReturnValuePatterns = new Dictionary (); + FeatureCheckReturnValuePatterns = new Dictionary (); Lattice = lattice; FeatureContextLattice = featureContextLattice; } @@ -93,10 +93,10 @@ public void Add (TrimAnalysisReflectionAccessPattern pattern) ReflectionAccessPatterns[pattern.Operation] = pattern.Merge (Lattice, FeatureContextLattice, existingPattern); } - public void Add (TrimAnalysisReturnValuePattern pattern) + public void Add (FeatureCheckReturnValuePattern pattern) { - if (!ReturnValuePatterns.TryGetValue (pattern.Operation, out var existingPattern)) { - ReturnValuePatterns.Add (pattern.Operation, pattern); + if (!FeatureCheckReturnValuePatterns.TryGetValue (pattern.Operation, out var existingPattern)) { + FeatureCheckReturnValuePatterns.Add (pattern.Operation, pattern); return; } @@ -130,7 +130,7 @@ public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext conte yield return diagnostic; } - foreach (var returnValuePattern in ReturnValuePatterns.Values) { + foreach (var returnValuePattern in FeatureCheckReturnValuePatterns.Values) { foreach (var diagnostic in returnValuePattern.CollectDiagnostics (context)) yield return diagnostic; } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs deleted file mode 100644 index 7bd30c31a85a18..00000000000000 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisReturnValuePattern.cs.cs +++ /dev/null @@ -1,68 +0,0 @@ -// 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 ILLink.Shared; -using ILLink.Shared.DataFlow; -using ILLink.Shared.TrimAnalysis; -using ILLink.RoslynAnalyzer.DataFlow; -using Microsoft.CodeAnalysis; - -namespace ILLink.RoslynAnalyzer.TrimAnalysis -{ - public readonly record struct TrimAnalysisReturnValuePattern - { - public FeatureChecksValue ReturnValue { get; init; } - public ValueSet FeatureCheckAnnotations { get; init; } - public IOperation Operation { get; init; } - public IPropertySymbol OwningSymbol { get; init; } - - public TrimAnalysisReturnValuePattern ( - FeatureChecksValue returnValue, - ValueSet featureCheckAnnotations, - IOperation operation, - IPropertySymbol owningSymbol) - { - ReturnValue = returnValue.DeepCopy (); - FeatureCheckAnnotations = featureCheckAnnotations.DeepCopy (); - Operation = operation; - OwningSymbol = owningSymbol; - } - - public IEnumerable CollectDiagnostics (DataFlowAnalyzerContext context) - { - var diagnosticContext = new DiagnosticContext (Operation.Syntax.GetLocation ()); - // For now, feature check validation is enabled only when trim analysis is enabled. - if (context.EnableTrimAnalyzer) { - if (!OwningSymbol.IsStatic || OwningSymbol.Type.SpecialType != SpecialType.System_Boolean) { - // Warn about invalid feature checks (non-static or non-bool properties) - diagnosticContext.AddDiagnostic ( - DiagnosticId.InvalidFeatureCheck); - return diagnosticContext.Diagnostics; - } - - if (ReturnValue != FeatureChecksValue.All) { - ValueSet returnValueFeatures = ReturnValue.EnabledFeatures; - // For any analyzer-supported feature that this property is declared to guard, - // the abstract return value must include that feature - // (indicating it is known to be enabled when the return value is true). - foreach (string feature in FeatureCheckAnnotations.GetKnownValues ()) { - foreach (var analyzer in context.EnabledRequiresAnalyzers) { - if (feature != analyzer.RequiresAttributeFullyQualifiedName) - continue; - - if (!returnValueFeatures.Contains (feature)) { - diagnosticContext.AddDiagnostic ( - DiagnosticId.ReturnValueDoesNotMatchFeatureChecks, - OwningSymbol.GetDisplayName (), - feature); - } - } - } - } - } - - return diagnosticContext.Diagnostics; - } - } -} diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs index 7d324187d1b817..04efbb63ff5bda 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs @@ -448,7 +448,7 @@ public override void HandleReturnConditionValue (FeatureChecksValue returnCondit return; TrimAnalysisPatterns.Add ( - new TrimAnalysisReturnValuePattern (returnConditionValue, featureCheckAnnotations, operation, propertySymbol)); + new FeatureCheckReturnValuePattern (returnConditionValue, featureCheckAnnotations, operation, propertySymbol)); } public override MultiValue HandleDelegateCreation (IMethodSymbol method, IOperation operation, in FeatureContext featureContext) diff --git a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx index ae3d0f63b27afc..ae50b7dcd9fff6 100644 --- a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx +++ b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx @@ -1201,7 +1201,7 @@ Return value does not match FeatureCheckAttribute '{1}'. - Return value does not match feature check annotations of the property. + Return value does not match FeatureCheck annotations of the property. The check should return false whenever any of the features referenced in the FeatureCheck annotations is disabled. Invalid FeatureCheckAttribute. The attribute must be placed on a static boolean property with only a 'get' accessor.