Skip to content

Commit

Permalink
Don't emit analyzer warnings for MakeGeneric understood by ILC (#100858)
Browse files Browse the repository at this point in the history
Follow up to #99037.
Resolves #81204.

When `MakeGenericXXX` API is used in the "bridging constraints" pattern and all types/members involved are statically known, don't emit warning from the Roslyn analyzer since ILC will do the right thing and ensure this works.
  • Loading branch information
MichalStrehovsky authored Apr 12, 2024
1 parent 324bd56 commit 7b534da
Show file tree
Hide file tree
Showing 7 changed files with 316 additions and 2 deletions.
10 changes: 10 additions & 0 deletions src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>;

namespace ILLink.RoslynAnalyzer
{
Expand Down Expand Up @@ -342,5 +343,14 @@ internal bool CheckAndCreateRequiresDiagnostic (
incompatibleMembers,
out diagnostic);
}

internal virtual bool IsIntrinsicallyHandled (
IMethodSymbol calledMethod,
MultiValue instance,
ImmutableArray<MultiValue> arguments
)
{
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
using System.Collections.Immutable;
using System.Linq;
using ILLink.Shared;
using ILLink.Shared.TrimAnalysis;
using ILLink.Shared.TypeSystemProxy;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>;

namespace ILLink.RoslynAnalyzer
{
Expand Down Expand Up @@ -38,6 +41,77 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase
internal override bool IsAnalyzerEnabled (AnalyzerOptions options) =>
options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAotAnalyzer);

internal override bool IsIntrinsicallyHandled (IMethodSymbol calledMethod, MultiValue instance, ImmutableArray<MultiValue> arguments) {
MethodProxy method = new (calledMethod);
var intrinsicId = Intrinsics.GetIntrinsicIdForMethod (method);

switch (intrinsicId) {
case IntrinsicId.Type_MakeGenericType: {
if (!instance.IsEmpty ()) {
foreach (var value in instance.AsEnumerable ()) {
if (value is SystemTypeValue) {
if (!IsKnownInstantiation (arguments[0])) {
return false;
}
} else {
return false;
}
}
}
return true;
}
case IntrinsicId.MethodInfo_MakeGenericMethod: {
if (!instance.IsEmpty ()) {
foreach (var methodValue in instance.AsEnumerable ()) {
if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) {
if (!IsKnownInstantiation (arguments[0])) {
return false;
}
} else {
return false;
}
}
}
return true;
}
}

return false;

static bool IsKnownInstantiation(MultiValue genericParametersArray) {
var typesValue = genericParametersArray.AsSingleValue ();
if (typesValue is NullValue) {
// This will fail at runtime but no warning needed
return true;
}

// Is this an array we model?
if (typesValue is not ArrayValue array) {
return false;
}

int? size = array.Size.AsConstInt ();
if (size == null) {
return false;
}

for (int i = 0; i < size.Value; i++) {
// Go over each element of the array. If the value is unknown, bail.
if (!array.TryGetValueByIndex (i, out MultiValue value)) {
return false;
}

var singleValue = value.AsSingleValue ();

if (singleValue is not SystemTypeValue and not GenericParameterValue and not NullableSystemTypeValue) {
return false;
}
}

return true;
}
}

private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol, Compilation compilation) {
var runtimeFeaturesType = compilation.GetTypeByMetadataName ("System.Runtime.CompilerServices.RuntimeFeature");
if (runtimeFeaturesType == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,11 @@ public IEnumerable<Diagnostic> CollectDiagnostics (DataFlowAnalyzerContext conte

foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers)
{
if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic(Operation, CalledMethod, OwningSymbol, context, FeatureContext, out Diagnostic? diag))
diagnosticContext.AddDiagnostic(diag);
if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, CalledMethod, OwningSymbol, context, FeatureContext, out Diagnostic? diag)
&& !requiresAnalyzer.IsIntrinsicallyHandled (CalledMethod, Instance, Arguments))
{
diagnosticContext.AddDiagnostic (diag);
}
}

return diagnosticContext.Diagnostics;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,12 @@ public Task MakeGenericDataFlow ()
return RunTest ();
}

[Fact]
public Task MakeGenericDataflowIntrinsics ()
{
return RunTest ();
}

[Fact]
public Task MethodByRefReturnDataFlow ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using ILLink.Shared;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Testing;
using Microsoft.CodeAnalysis.Text;
using Xunit;
Expand Down Expand Up @@ -34,6 +36,19 @@ public RequiresDynamicCodeAttribute (string message)
}
}";

static async Task VerifyRequiresDynamicCodeAnalyzer (
string source,
params DiagnosticResult[] expected)
{

await VerifyCS.VerifyAnalyzerAsync (
source,
consoleApplication: false,
TestCaseUtils.UseMSBuildProperties (MSBuildPropertyOptionNames.EnableAotAnalyzer),
Array.Empty<MetadataReference> (),
expected);
}

static Task VerifyRequiresDynamicCodeCodeFix (
string source,
string fixedSource,
Expand Down Expand Up @@ -332,5 +347,133 @@ private int M2 {
};
return VerifyRequiresDynamicCodeCodeFix (src, fix, diag, Array.Empty<DiagnosticResult> ());
}

[Fact]
public Task MakeGenericTypeWithAllKnownTypes ()
{
const string src = $$"""
class C
{
public void M() => typeof(Gen<>).MakeGenericType(typeof(object));
}
class Gen<T> { }
""";

return VerifyRequiresDynamicCodeAnalyzer (src);
}

[Fact]
public Task MakeGenericTypeWithAllKnownTypesInGenericContext ()
{
const string src = $$"""
class C
{
public void M<T>() => typeof(Gen<>).MakeGenericType(typeof(T));
}
class Gen<T> { }
""";

return VerifyRequiresDynamicCodeAnalyzer (src);
}

[Fact]
public Task MakeGenericTypeWithUnknownDefinition ()
{
const string src = $$"""
using System;
class C
{
public void M() => GetDefinition().MakeGenericType(typeof(object));
static Type GetDefinition() => typeof(Gen<>);
}
class Gen<T> { }
""";

return VerifyRequiresDynamicCodeAnalyzer (src,
// (4,21): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 68).WithArguments ("System.Type.MakeGenericType(params Type[])", " The native code for this instantiation might not be available at runtime.", ""));
}

[Fact]
public Task MakeGenericTypeWithUnknownArgument ()
{
const string src = $$"""
using System;
class C
{
public void M() => typeof(Gen<>).MakeGenericType(GetObject());
static Type GetObject() => typeof(object);
}
class Gen<T> { }
""";

return VerifyRequiresDynamicCodeAnalyzer (src,
// (4,21): warning IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 63).WithArguments ("System.Type.MakeGenericType(params Type[])", " The native code for this instantiation might not be available at runtime.", ""));
}

[Fact]
public Task MakeGenericMethodWithAllKnownTypes ()
{
const string src = $$"""
class C
{
public void M() => typeof(C).GetMethod(nameof(N)).MakeGenericMethod(typeof(object));
public void N<T>() { }
}
""";

return VerifyRequiresDynamicCodeAnalyzer (src);
}

[Fact]
public Task MakeGenericMethodWithAllKnownTypesInGenericContext ()
{
const string src = $$"""
class C
{
public void M<T>() => typeof(C).GetMethod(nameof(N)).MakeGenericMethod(typeof(T));
public void N<T>() { }
}
""";

return VerifyRequiresDynamicCodeAnalyzer (src);
}

[Fact]
public Task MakeGenericMethodWithUnknownDefinition ()
{
const string src = $$"""
using System.Reflection;
class C
{
public void M() => GetMethodInfo().MakeGenericMethod(typeof(object));
public void N<T>() { }
public MethodInfo GetMethodInfo() => typeof(C).GetMethod(nameof(N));
}
""";

return VerifyRequiresDynamicCodeAnalyzer (src,
// (4,21): warning IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 70).WithArguments ("System.Reflection.MethodInfo.MakeGenericMethod(params Type[])", " The native code for this instantiation might not be available at runtime.", ""));
}

[Fact]
public Task MakeGenericMethodWithUnknownArgument ()
{
const string src = $$"""
using System;
class C
{
public void M() => typeof(C).GetMethod(nameof(N)).MakeGenericMethod(GetObject());
public void N<T>() { }
static Type GetObject() => typeof(object);
}
""";

return VerifyRequiresDynamicCodeAnalyzer (src,
// (4,21): warning IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
VerifyCS.Diagnostic (DiagnosticId.RequiresDynamicCode).WithSpan (4, 21, 4, 82).WithArguments ("System.Reflection.MethodInfo.MakeGenericMethod(params Type[])", " The native code for this instantiation might not be available at runtime.", ""));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ public Task GenericParameterDataFlowMarking ()
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task MakeGenericDataflowIntrinsics ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task MethodByRefParameterDataFlow ()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using Mono.Linker.Tests.Cases.Expectations.Assertions;

namespace Mono.Linker.Tests.Cases.DataFlow
{
[SkipKeptItemsValidation]
[ExpectedNoWarnings]
class MakeGenericDataflowIntrinsics
{
public static void Main ()
{
MakeGenericType.Test ();
MakeGenericMethod.Test ();
}

class MakeGenericType
{
class Gen<T> { }

static Type GrabUnknownType () => null;

public static void Test ()
{
TestRecognizedIntrinsic ();
TestRecognizedGenericIntrinsic<object> ();
TestUnknownOwningType ();
TestUnknownArgument ();
}

public static void TestRecognizedIntrinsic () => typeof (Gen<>).MakeGenericType (typeof (object));

public static void TestRecognizedGenericIntrinsic<T> () => typeof (Gen<>).MakeGenericType (typeof (T));

[ExpectedWarning ("IL2055", nameof (Type.MakeGenericType))]
[ExpectedWarning ("IL3050", nameof (Type.MakeGenericType), ProducedBy = Tool.Analyzer | Tool.NativeAot)]
public static void TestUnknownOwningType () => GrabUnknownType ().MakeGenericType (typeof (object));

[ExpectedWarning ("IL3050", nameof (Type.MakeGenericType), ProducedBy = Tool.Analyzer | Tool.NativeAot)]
public static void TestUnknownArgument () => typeof (Gen<>).MakeGenericType (GrabUnknownType ());
}

class MakeGenericMethod
{
public static void Gen<T> () { }

static MethodInfo GrabUnknownMethod () => null;

static Type GrabUnknownType () => null;

public static void Test ()
{
TestRecognizedIntrinsic ();
TestRecognizedGenericIntrinsic<object> ();
TestUnknownOwningMethod ();
TestUnknownArgument ();
}

public static void TestRecognizedIntrinsic () => typeof (MakeGenericMethod).GetMethod (nameof (Gen)).MakeGenericMethod (typeof (object));

public static void TestRecognizedGenericIntrinsic<T> () => typeof (MakeGenericMethod).GetMethod (nameof (Gen)).MakeGenericMethod (typeof (T));

[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod))]
[ExpectedWarning ("IL3050", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Analyzer | Tool.NativeAot)]
public static void TestUnknownOwningMethod () => GrabUnknownMethod ().MakeGenericMethod (typeof (object));

[ExpectedWarning ("IL3050", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Analyzer | Tool.NativeAot)]
public static void TestUnknownArgument () => typeof (MakeGenericMethod).GetMethod (nameof (Gen)).MakeGenericMethod (GrabUnknownType());
}
}
}

0 comments on commit 7b534da

Please sign in to comment.