-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add ILLink/ILCompiler support for feature checks #99267
Changes from all commits
8961369
455e088
62fe4dd
f0e2383
4ab6d5a
841fec7
b036a92
3dbdbef
5973211
f4498d1
7319e55
c46aa29
b6bf7c6
f5cc166
70918e8
a1f5907
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Reflection; | ||
using System.Reflection.Metadata; | ||
using System.Reflection.PortableExecutable; | ||
using System.Resources; | ||
|
@@ -33,11 +34,84 @@ public BodySubstitution GetSubstitution(MethodDesc method) | |
AssemblyFeatureInfo info = _hashtable.GetOrCreateValue(ecmaMethod.Module); | ||
if (info.BodySubstitutions != null && info.BodySubstitutions.TryGetValue(ecmaMethod, out BodySubstitution result)) | ||
return result; | ||
|
||
if (TryGetFeatureCheckValue(ecmaMethod, out bool value)) | ||
return BodySubstitution.Create(value ? 1 : 0); | ||
} | ||
|
||
return null; | ||
} | ||
|
||
private bool TryGetFeatureCheckValue(EcmaMethod method, out bool value) | ||
{ | ||
value = false; | ||
|
||
if (!method.Signature.IsStatic) | ||
return false; | ||
|
||
if (!method.Signature.ReturnType.IsWellKnownType(WellKnownType.Boolean)) | ||
return false; | ||
|
||
if (FindProperty(method) is not PropertyPseudoDesc property) | ||
return false; | ||
|
||
if (property.SetMethod != null) | ||
return false; | ||
|
||
foreach (var featureSwitchDefinitionAttribute in property.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "FeatureSwitchDefinitionAttribute")) | ||
{ | ||
if (featureSwitchDefinitionAttribute.FixedArguments is not [CustomAttributeTypedArgument<TypeDesc> { Value: string switchName }]) | ||
continue; | ||
|
||
// If there's a FeatureSwitchDefinition, don't continue looking for FeatureGuard. | ||
// We don't want to infer feature switch settings from FeatureGuard. | ||
return _hashtable._switchValues.TryGetValue(switchName, out value); | ||
} | ||
|
||
foreach (var featureGuardAttribute in property.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "FeatureGuardAttribute")) | ||
{ | ||
if (featureGuardAttribute.FixedArguments is not [CustomAttributeTypedArgument<TypeDesc> { Value: EcmaType featureType }]) | ||
continue; | ||
|
||
if (featureType.Namespace == "System.Diagnostics.CodeAnalysis") { | ||
switch (featureType.Name) { | ||
case "RequiresAssemblyFilesAttribute": | ||
case "RequiresUnreferencedCodeAttribute": | ||
return true; | ||
case "RequiresDynamicCodeAttribute": | ||
if (_hashtable._switchValues.TryGetValue( | ||
"System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", | ||
out bool isDynamicCodeSupported) | ||
&& !isDynamicCodeSupported) | ||
return true; | ||
Comment on lines
+82
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this? IsDynamicCodeSupported/Compiled is hardcoded to false in native AOT corelib. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did this to match the behavior of some substitutions I noticed for System.Linq, that only kick in when the feature is explicitly passed on the command-line. It's kind of a moot point unless somebody tries to set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! |
||
break; | ||
} | ||
} | ||
} | ||
|
||
return false; | ||
|
||
static PropertyPseudoDesc FindProperty(EcmaMethod method) | ||
{ | ||
if ((method.Attributes & MethodAttributes.SpecialName) == 0) | ||
return null; | ||
|
||
if (method.OwningType is not EcmaType declaringType) | ||
return null; | ||
|
||
var reader = declaringType.MetadataReader; | ||
foreach (PropertyDefinitionHandle propertyHandle in reader.GetTypeDefinition(declaringType.Handle).GetProperties()) | ||
{ | ||
PropertyDefinition propertyDef = reader.GetPropertyDefinition(propertyHandle); | ||
var property = new PropertyPseudoDesc(declaringType, propertyHandle); | ||
if (property.GetMethod == method) | ||
return property; | ||
} | ||
|
||
return null; | ||
} | ||
} | ||
|
||
public object GetSubstitution(FieldDesc field) | ||
{ | ||
if (field.GetTypicalFieldDefinition() is EcmaField ecmaField) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
using System; | ||
using System.Threading.Tasks; | ||
using Xunit; | ||
|
||
namespace ILLink.RoslynAnalyzer.Tests | ||
{ | ||
public sealed partial class SubstitutionsTests : LinkerTestBase | ||
{ | ||
protected override string TestSuiteName => "Substitutions"; | ||
|
||
[Fact] | ||
public Task FeatureGuardSubstitutions () | ||
{ | ||
return RunTest (); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double checking that we don't allow feature types to be generic/arrays/pointers (non-definition type will never be EcmaType).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature types aren't restricted in any way, but everything will be a no-op except for
RequiresUnreferencedCodeAttribute
,RequiresDynamicCodeAttribute
, andRequiresAssemblyFilesAttribute
. A third-party analyzer could theoretically add support for other types (including generics) so we don't block other types.