Skip to content

Commit def63fe

Browse files
authored
[release/10.0] Add trim analyzer support for C# 14 extensions (#119195)
* Add trim analyzer support for C# 14 extensions (#119017) Fixes #115949 This adds Roslyn analyzer support for C# 14 extensions. A few notes on the behavior: - DAM is supported on method return/params, including on property accessor return/param. - DAM is not supported on extension properties (see #119113) - DAM is not supported on extension methods (this produces IL2041) - There's still a bug with the behavior for ILLink/ILC due to the annotations not being preserved into the implementation when lowering: dotnet/roslyn#80017. Includes an update to Microsoft.CodeAnalysis for new analyzer APIs involving extension members. #119159 tracks fixing new warnings that are suppressed for now. New tests uncovered an analyzer issue with operators in the (extension or not): #119110. This change includes test coverage for non-extension operators. They also uncovered an issue that looks like a race condition in ILC: #119155. * Fix indentation
1 parent 728cf42 commit def63fe

File tree

18 files changed

+616
-43
lines changed

18 files changed

+616
-43
lines changed

Directory.Build.props

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,10 @@
269269
<WarningsNotAsErrors Condition="'$(OfficialBuild)' != 'true' OR '$(NuGetAuditWarnNotError)' == 'true'">$(WarningsNotAsErrors);NU1901;NU1902;NU1903;NU1904</WarningsNotAsErrors>
270270
<!-- Warnings to always disable -->
271271
<NoWarn>$(NoWarn);CS8500;CS8969</NoWarn>
272+
<!-- Don't warn about redundant equality -->
273+
<NoWarn>$(NoWarn);IDE0100</NoWarn>
274+
<!-- Don't warn about unused parameters -->
275+
<NoWarn>$(NoWarn);IDE0060</NoWarn>
272276
<!-- Suppress "CS1591 - Missing XML comment for publicly visible type or member" compiler errors for private assemblies. -->
273277
<NoWarn Condition="'$(IsPrivateAssembly)' == 'true'">$(NoWarn);CS1591</NoWarn>
274278
<!-- Always pass portable to override arcade sdk which uses embedded for local builds -->

eng/Versions.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
Source-build builds the product with the most recent previously source-built release. Thankfully, these two requirements line up nicely
5757
such that any version that satisfies the VS version requirement will also satisfy the .NET SDK version requirement because of how we ship.
5858
-->
59-
<MicrosoftCodeAnalysisVersion_LatestVS>4.8.0</MicrosoftCodeAnalysisVersion_LatestVS>
59+
<MicrosoftCodeAnalysisVersion_LatestVS>4.14.0</MicrosoftCodeAnalysisVersion_LatestVS>
6060
<!-- Some of the analyzer dependencies used by ILLink project -->
6161
<MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.5-beta1.23270.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
6262
</PropertyGroup>

src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,20 @@ private void VerifyLoggedMessages(AssemblyDefinition original, TrimmingTestLogge
202202
List<(ICustomAttributeProvider, CustomAttribute)> expectedNoWarningsAttributes = new();
203203
foreach (var attrProvider in GetAttributeProviders(original))
204204
{
205+
if (attrProvider is IMemberDefinition attrMember &&
206+
attrMember is not TypeDefinition &&
207+
attrMember.DeclaringType is TypeDefinition declaringType &&
208+
declaringType.Name.StartsWith("<G>"))
209+
{
210+
// Workaround: C# 14 extension members result in a compiler-generated type
211+
// that has a member for each extension member (this is in addition to the type
212+
// which contains the actual extension member implementation).
213+
// The generated members inherit attributes from the extension members, but
214+
// have empty implementations. We don't want to check inherited ExpectedWarningAttributes
215+
// for these members.
216+
continue;
217+
}
218+
205219
foreach (var attr in attrProvider.CustomAttributes)
206220
{
207221
if (!IsProducedByNativeAOT(attr))

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,16 @@ bool merge
305305
return value;
306306
}
307307

308-
// Property may be an indexer, in which case there will be one or more index arguments followed by a value argument
309308
ImmutableArray<TValue>.Builder arguments = ImmutableArray.CreateBuilder<TValue>();
309+
310+
// Handle C# 14 extension property access (see comment in ProcessMethodCall)
311+
if (setMethod.HasExtensionParameterOnType())
312+
{
313+
arguments.Add(instanceValue);
314+
instanceValue = TopValue;
315+
}
316+
317+
// Property may be an indexer, in which case there will be one or more index arguments followed by a value argument
310318
foreach (var val in propertyRef.Arguments)
311319
arguments.Add(Visit(val, state));
312320
arguments.Add(value);
@@ -705,14 +713,20 @@ public override TValue VisitPropertyReference(IPropertyReferenceOperation operat
705713
// Accessing property for reading is really a call to the getter
706714
// The setter case is handled in assignment operation since here we don't have access to the value to pass to the setter
707715
TValue instanceValue = Visit(operation.Instance, state);
708-
IMethodSymbol? getMethod = operation.Property.GetGetMethod();
716+
IMethodSymbol getMethod = operation.Property.GetGetMethod()!;
717+
ImmutableArray<TValue>.Builder arguments = ImmutableArray.CreateBuilder<TValue>();
718+
// Handle C# 14 extension property access (see comment in ProcessMethodCall)
719+
if (getMethod.HasExtensionParameterOnType())
720+
{
721+
arguments.Add(instanceValue);
722+
instanceValue = TopValue;
723+
}
709724

710725
// Property may be an indexer, in which case there will be one or more index arguments
711-
ImmutableArray<TValue>.Builder arguments = ImmutableArray.CreateBuilder<TValue>();
712726
foreach (var val in operation.Arguments)
713727
arguments.Add(Visit(val, state));
714728

715-
return HandleMethodCallHelper(getMethod!, instanceValue, arguments.ToImmutableArray(), operation, state);
729+
return HandleMethodCallHelper(getMethod, instanceValue, arguments.ToImmutableArray(), operation, state);
716730
}
717731

718732
public override TValue VisitEventReference(IEventReferenceOperation operation, LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
@@ -890,7 +904,7 @@ private TValue HandleMethodCallHelper(
890904
TConditionValue conditionValue = GetConditionValue(argumentOperation, state);
891905
var current = state.Current;
892906
ApplyCondition(
893-
doesNotReturnIfConditionValue == false
907+
!doesNotReturnIfConditionValue
894908
? conditionValue
895909
: conditionValue.Negate(),
896910
ref current);
@@ -910,6 +924,16 @@ private TValue ProcessMethodCall(
910924
TValue instanceValue = Visit(instance, state);
911925

912926
var argumentsBuilder = ImmutableArray.CreateBuilder<TValue>();
927+
928+
// For calls to C# 14 extensions, treat the instance argument as a regular argument.
929+
// The extension method doesn't have an implicit this, yet (unlike for existing extension methods)
930+
// the IOperation represents it as having an Instance.
931+
if (method.HasExtensionParameterOnType())
932+
{
933+
argumentsBuilder.Add(instanceValue);
934+
instanceValue = TopValue;
935+
}
936+
913937
foreach (var argument in arguments)
914938
{
915939
// For __arglist argument there might not be any parameter

src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,11 @@ private static void VerifyDamOnPropertyAndAccessorMatch(SymbolAnalysisContext co
322322
|| propertySymbol.GetDynamicallyAccessedMemberTypes() == DynamicallyAccessedMemberTypes.None)
323323
return;
324324

325+
// For C# 14 extension properties, property-level DAM does not propagate to accessors
326+
// and we do not consider property vs accessor conflicts meaningful. Skip conflict checks.
327+
if (methodSymbol.HasExtensionParameterOnType())
328+
return;
329+
325330
// None on the return type of 'get' matches unannotated
326331
if (methodSymbol.MethodKind == MethodKind.PropertyGet
327332
&& methodSymbol.GetDynamicallyAccessedMemberTypesOnReturnType() != DynamicallyAccessedMemberTypes.None

src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
<NoWarn Condition="'$(DotNetBuildSourceOnly)' == 'true'">$(NoWarn);CS8524</NoWarn>
1111
<!-- Type conflicts with imported type. Silence this for source build to allow building with the latest
1212
source-included type name parser, instead of the one that comes with Roslyn. -->
13-
<NoWarn Condition="'$(DotNetBuildSourceOnly)' == 'true'">$(NoWarn);CS0436</NoWarn>
13+
<NoWarn>$(NoWarn);CS0436</NoWarn>
1414
<AnalyzerLanguage>cs</AnalyzerLanguage>
1515
<!-- The analyzer needs to process deeply nested expressions in corelib.
1616
This can blow up the stack if using unoptimized code (due to large

src/tools/illink/src/ILLink.RoslynAnalyzer/IMethodSymbolExtensions.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,23 @@ public static ParameterProxy GetParameter(this IMethodSymbol method, ParameterIn
5858
/// </summary>
5959
public static int GetMetadataParametersCount(this IMethodSymbol method)
6060
{
61-
return method.Parameters.Length;
61+
return method.Parameters.Length + (method.HasExtensionParameterOnType() ? 1 : 0);
6262
}
6363

6464
/// <summary>
6565
/// Gets the number of parameters pushed onto the stack when the method is called (including the implicit 'this' paramter)
6666
/// </summary>
6767
public static int GetParametersCount(this IMethodSymbol method)
6868
{
69-
return method.Parameters.Length + (method.HasImplicitThis() ? 1 : 0);
69+
return method.Parameters.Length + (method.HasImplicitThis() ? 1 : 0) + (method.HasExtensionParameterOnType() ? 1 : 0);
70+
}
71+
72+
/// <summary>
73+
/// Returns true if the method's containing type has an extension parameter
74+
/// </summary>
75+
public static bool HasExtensionParameterOnType(this IMethodSymbol method)
76+
{
77+
return method.ContainingType.ExtensionParameter != null;
7078
}
7179
}
7280
}

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FlowAnnotations.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,16 +165,17 @@ internal static DynamicallyAccessedMemberTypes GetMethodParameterAnnotation(Para
165165

166166
var damt = parameter.GetDynamicallyAccessedMemberTypes();
167167

168-
var parameterMethod = (IMethodSymbol)parameter.ContainingSymbol;
169-
Debug.Assert(parameterMethod != null);
168+
IMethodSymbol parameterMethod = param.Method.Method;
170169

171170
// If there are conflicts between the setter and the property annotation,
172171
// the setter annotation wins. (But DAMT.None is ignored)
173172

174173
// Is this a property setter `value` parameter?
175174
if (parameterMethod!.MethodKind == MethodKind.PropertySet
176175
&& damt == DynamicallyAccessedMemberTypes.None
177-
&& parameter.Ordinal == parameterMethod.Parameters.Length - 1)
176+
&& parameter.Ordinal == parameterMethod.Parameters.Length - 1
177+
// Do not propagate property-level DAM for C# 14 extension properties
178+
&& !parameterMethod.HasExtensionParameterOnType())
178179
{
179180
var property = (IPropertySymbol)parameterMethod.AssociatedSymbol!;
180181
Debug.Assert(property != null);
@@ -194,7 +195,10 @@ public static DynamicallyAccessedMemberTypes GetMethodReturnValueAnnotation(IMet
194195
// Is this a property getter?
195196
// If there are conflicts between the getter and the property annotation,
196197
// the getter annotation wins. (But DAMT.None is ignored)
197-
if (method.MethodKind is MethodKind.PropertyGet && returnDamt == DynamicallyAccessedMemberTypes.None)
198+
if (method.MethodKind is MethodKind.PropertyGet
199+
&& returnDamt == DynamicallyAccessedMemberTypes.None
200+
// Do not propagate property-level DAM for C# 14 extension properties]
201+
&& !method.HasExtensionParameterOnType())
198202
{
199203
var property = (IPropertySymbol)method.AssociatedSymbol!;
200204
Debug.Assert(property != null);

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/MethodParameterValue.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,6 @@ namespace ILLink.Shared.TrimAnalysis
99
{
1010
internal partial record MethodParameterValue
1111
{
12-
public MethodParameterValue(IParameterSymbol parameterSymbol)
13-
: this(new ParameterProxy(parameterSymbol)) { }
14-
public MethodParameterValue(IMethodSymbol methodSymbol, ParameterIndex parameterIndex, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
15-
: this(new(new(methodSymbol), parameterIndex), dynamicallyAccessedMemberTypes) { }
16-
1712
public MethodParameterValue(ParameterProxy parameter)
1813
: this(parameter, FlowAnnotations.GetMethodParameterAnnotation(parameter)) { }
1914

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ParameterProxy.cs

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ namespace ILLink.Shared.TypeSystemProxy
99
{
1010
internal partial struct ParameterProxy
1111
{
12-
public ParameterProxy(IParameterSymbol parameter)
12+
public ParameterProxy(IParameterSymbol parameter, IMethodSymbol method)
1313
{
14-
Method = new((IMethodSymbol)parameter.ContainingSymbol);
15-
Index = (ParameterIndex)parameter.Ordinal + (Method.HasImplicitThis() ? 1 : 0);
14+
Method = new(method);
15+
Index = (ParameterIndex)parameter.Ordinal +
16+
(Method.HasImplicitThis() ? 1 : 0) +
17+
(method.HasExtensionParameterOnType() && parameter.ContainingSymbol is IMethodSymbol ? 1 : 0);
1618
}
1719

1820
public partial ReferenceKind GetReferenceKind()
@@ -22,32 +24,51 @@ public partial ReferenceKind GetReferenceKind()
2224
? ReferenceKind.Ref
2325
: ReferenceKind.None;
2426

25-
switch (Method.Method.Parameters[MetadataIndex].RefKind)
27+
switch (ParameterSymbol!.RefKind)
2628
{
2729
case RefKind.Ref: return ReferenceKind.Ref;
2830
case RefKind.In: return ReferenceKind.In;
2931
case RefKind.Out: return ReferenceKind.Out;
3032
case RefKind.None: return ReferenceKind.None;
3133
default:
32-
Debug.Fail($"Unexpected RefKind {Method.Method.Parameters[MetadataIndex].RefKind} found on parameter {GetDisplayName()}");
34+
Debug.Fail($"Unexpected RefKind {ParameterSymbol!.RefKind} found on parameter {GetDisplayName()}");
3335
return ReferenceKind.None;
3436
}
3537
}
3638

3739
/// <summary>
3840
/// Returns the IParameterSymbol representing the parameter. Returns null for the implicit this paramter.
3941
/// </summary>
40-
public IParameterSymbol? ParameterSymbol => IsImplicitThis ? null : Method.Method.Parameters[MetadataIndex];
42+
public IParameterSymbol? ParameterSymbol
43+
{
44+
get
45+
{
46+
if (IsImplicitThis)
47+
{
48+
return null;
49+
}
50+
51+
var method = Method.Method;
52+
int hasExtensionParameter = 0;
53+
if (method.HasExtensionParameterOnType())
54+
{
55+
if (MetadataIndex == 0)
56+
{
57+
return method.ContainingType.ExtensionParameter;
58+
}
59+
hasExtensionParameter = 1;
60+
}
61+
62+
return method.Parameters[MetadataIndex - hasExtensionParameter];
63+
}
64+
}
4165

4266
/// <summary>
4367
/// Returns the IParameterSymbol.Location[0] for the parameter. Returns null for the implicit this paramter.
4468
/// </summary>
4569
public Location? Location => ParameterSymbol?.Locations[0];
4670

47-
public TypeProxy ParameterType
48-
=> IsImplicitThis
49-
? new TypeProxy(Method.Method.ContainingType)
50-
: new TypeProxy(Method.Method.Parameters[MetadataIndex].Type);
71+
public TypeProxy ParameterType => new TypeProxy(ParameterSymbol?.Type ?? Method.Method.ContainingType);
5172

5273
public partial string GetDisplayName()
5374
{

0 commit comments

Comments
 (0)