Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special case MakeGenericMethod/Type in RUC analyzer #2209

Merged
merged 5 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
static readonly DiagnosticDescriptor s_dynamicTypeInvocationRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresUnreferencedCode,
new LocalizableResourceString (nameof (SharedStrings.DynamicTypeInvocationTitle), SharedStrings.ResourceManager, typeof (SharedStrings)),
new LocalizableResourceString (nameof (SharedStrings.DynamicTypeInvocationMessage), SharedStrings.ResourceManager, typeof (SharedStrings)));
static readonly DiagnosticDescriptor s_makeGenericTypeRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.MakeGenericType);
static readonly DiagnosticDescriptor s_makeGenericMethodRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.MakeGenericMethod);

static readonly Action<OperationAnalysisContext> s_dynamicTypeInvocation = operationContext => {
if (FindContainingSymbol (operationContext, DiagnosticTargets.All) is ISymbol containingSymbol &&
Expand All @@ -32,7 +34,7 @@ public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
};

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create (s_dynamicTypeInvocationRule, s_requiresUnreferencedCodeRule, s_requiresUnreferencedCodeAttributeMismatch);
ImmutableArray.Create (s_dynamicTypeInvocationRule, s_makeGenericMethodRule, s_makeGenericTypeRule, s_requiresUnreferencedCodeRule, s_requiresUnreferencedCodeAttributeMismatch);

private protected override string RequiresAttributeName => RequiresUnreferencedCodeAttribute;

Expand All @@ -50,6 +52,32 @@ protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation
private protected override ImmutableArray<(Action<OperationAnalysisContext> Action, OperationKind[] OperationKind)> ExtraOperationActions =>
ImmutableArray.Create ((s_dynamicTypeInvocation, new OperationKind[] { OperationKind.DynamicInvocation }));

protected override ImmutableArray<ISymbol> GetSpecialIncompatibleMembers (Compilation compilation)
{
var incompatibleMembers = ImmutableArray.CreateBuilder<ISymbol> ();
var typeType = compilation.GetTypeByMetadataName ("System.Type");
if (typeType != null) {
incompatibleMembers.AddRange (typeType.GetMembers ("MakeGenericType").OfType<IMethodSymbol> ());
}

var methodInfoType = compilation.GetTypeByMetadataName ("System.Reflection.MethodInfo");
if (methodInfoType != null) {
incompatibleMembers.AddRange (methodInfoType.GetMembers ("MakeGenericMethod").OfType<IMethodSymbol> ());
}

return incompatibleMembers.ToImmutable ();
}

protected override bool ReportSpecialIncompatibleMembersDiagnostic (OperationAnalysisContext operationContext, ImmutableArray<ISymbol> specialIncompatibleMembers, ISymbol member)
{
if (member is IMethodSymbol method && ImmutableArrayOperations.Contains (specialIncompatibleMembers, member, SymbolEqualityComparer.Default) &&
(method.Name == "MakeGenericMethod" || method.Name == "MakeGenericType")) {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a comment why we're doing this?

}

return false;
}

protected override bool VerifyAttributeArguments (AttributeData attribute) =>
attribute.ConstructorArguments.Length >= 1 && attribute.ConstructorArguments[0] is { Type: { SpecialType: SpecialType.System_String } } ctorArg;

Expand Down
2 changes: 2 additions & 0 deletions src/ILLink.Shared/DiagnosticId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ public enum DiagnosticId
// Linker diagnostic ids.
RequiresUnreferencedCode = 2026,
RequiresUnreferencedCodeAttributeMismatch = 2046,
MakeGenericType = 2055,
MakeGenericMethod = 2060,
RequiresUnreferencedCodeOnStaticConstructor = 2116,

// Single-file diagnostic ids.
Expand Down
6 changes: 6 additions & 0 deletions src/ILLink.Shared/SharedStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@
<data name="DynamicTypeInvocationTitle" xml:space="preserve">
<value>Using dynamic types might cause types or members to be removed by trimmer.</value>
</data>
<data name="MakeGenericMethodMessage" xml:space="preserve">
<value>Call to '{0}' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic method.</value>
</data>
<data name="MakeGenericTypeMessage" xml:space="preserve">
<value>Call to '{0}' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic type.</value>
</data>
<data name="RequiresUnreferencedCodeOnStaticConstructorMessage" xml:space="preserve">
<value>'RequiresUnreferencedCodeAttribute' cannot be placed directly on static constructor '{0}', consider placing 'RequiresUnreferencedCodeAttribute' on the type declaration instead.</value>
</data>
Expand Down
33 changes: 13 additions & 20 deletions src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using ILLink.Shared;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Linker.Steps;
Expand Down Expand Up @@ -745,10 +746,8 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
}
}
if (hasUncheckedAnnotation) {
reflectionContext.RecordUnrecognizedPattern (
2055,
$"Call to '{calledMethodDefinition.GetDisplayName ()}' can not be statically analyzed. " +
$"It's not possible to guarantee the availability of requirements of the generic type.");
reflectionContext.RecordUnrecognizedPattern ((int) DiagnosticId.MakeGenericType,
new DiagnosticString (DiagnosticId.MakeGenericType).GetMessage (calledMethodDefinition.GetDisplayName ()));
}
}

Expand All @@ -758,10 +757,8 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
reflectionContext.RecordHandledPattern ();
else {
// We have no way to "include more" to fix this if we don't know, so we have to warn
reflectionContext.RecordUnrecognizedPattern (
2055,
$"Call to '{calledMethodDefinition.GetDisplayName ()}' can not be statically analyzed. " +
$"It's not possible to guarantee the availability of requirements of the generic type.");
reflectionContext.RecordUnrecognizedPattern ((int) DiagnosticId.MakeGenericType,
new DiagnosticString (DiagnosticId.MakeGenericType).GetMessage (calledMethodDefinition.GetDisplayName ()));
}
}

Expand Down Expand Up @@ -853,9 +850,8 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
if (hasTypeArguments) {
// We don't know what method the `MakeGenericMethod` was called on, so we have to assume
// that the method may have requirements which we can't fullfil -> warn.
reflectionContext.RecordUnrecognizedPattern (
2060, string.Format (Resources.Strings.IL2060,
DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod)));
reflectionContext.RecordUnrecognizedPattern ((int) DiagnosticId.MakeGenericMethod,
new DiagnosticString (DiagnosticId.MakeGenericMethod).GetMessage (DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod)));
}

RequireDynamicallyAccessedMembers (
Expand All @@ -869,9 +865,8 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
if (hasTypeArguments) {
// We don't know what method the `MakeGenericMethod` was called on, so we have to assume
// that the method may have requirements which we can't fullfil -> warn.
reflectionContext.RecordUnrecognizedPattern (
2060, string.Format (Resources.Strings.IL2060,
DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod)));
reflectionContext.RecordUnrecognizedPattern ((int) DiagnosticId.MakeGenericMethod,
new DiagnosticString (DiagnosticId.MakeGenericMethod).GetMessage (DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod)));
}

RequireDynamicallyAccessedMembers (
Expand Down Expand Up @@ -1718,9 +1713,8 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
} else {
// We don't know what method the `MakeGenericMethod` was called on, so we have to assume
// that the method may have requirements which we can't fullfil -> warn.
reflectionContext.RecordUnrecognizedPattern (
2060, string.Format (Resources.Strings.IL2060,
DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod)));
reflectionContext.RecordUnrecognizedPattern ((int) DiagnosticId.MakeGenericMethod,
new DiagnosticString (DiagnosticId.MakeGenericMethod).GetMessage (DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod)));
}
}

Expand Down Expand Up @@ -2403,9 +2397,8 @@ void ValidateGenericMethodInstantiation (
}

if (!AnalyzeGenericInstatiationTypeArray (genericParametersArray, ref reflectionContext, reflectionMethod, genericMethod.GenericParameters)) {
reflectionContext.RecordUnrecognizedPattern (
2060,
string.Format (Resources.Strings.IL2060, DiagnosticUtilities.GetMethodSignatureDisplayName (reflectionMethod)));
reflectionContext.RecordUnrecognizedPattern ((int) DiagnosticId.MakeGenericMethod,
new DiagnosticString (DiagnosticId.MakeGenericMethod).GetMessage (DiagnosticUtilities.GetMethodSignatureDisplayName (reflectionMethod)));
} else {
reflectionContext.RecordHandledPattern ();
}
Expand Down
9 changes: 0 additions & 9 deletions src/linker/Resources/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions src/linker/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="IL2060" xml:space="preserve">
<value>Call to '{0}' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic method.</value>
</data>
<data name="IL2067" xml:space="preserve">
<value>'{0}' argument does not satisfy {4} in call to '{1}'. The parameter '{2}' of method '{3}' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -920,5 +920,53 @@ static void M0 ()

return VerifyRequiresUnreferencedCodeAnalyzer (source);
}

[Fact]
public Task TestMakeGenericMethodUsage ()
{
var source = @"
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

class C
{
static void M1 (MethodInfo methodInfo)
{
methodInfo.MakeGenericMethod (typeof (C));
}

[RequiresUnreferencedCode (""Message from RUC"")]
static void M2 (MethodInfo methodInfo)
{
methodInfo.MakeGenericMethod (typeof (C));
}
}";

return VerifyRequiresUnreferencedCodeAnalyzer (source);
}

[Fact]
public Task TestMakeGenericTypeUsage ()
{
var source = @"
using System;
using System.Diagnostics.CodeAnalysis;

class C
{
static void M1 (Type t)
{
typeof (Nullable<>).MakeGenericType (typeof (C));
}

[RequiresUnreferencedCode (""Message from RUC"")]
static void M2 (Type t)
{
typeof (Nullable<>).MakeGenericType (typeof (C));
}
}";

return VerifyRequiresUnreferencedCodeAnalyzer (source);
}
}
}