Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@
<WarningsNotAsErrors Condition="'$(OfficialBuild)' != 'true' OR '$(NuGetAuditWarnNotError)' == 'true'">$(WarningsNotAsErrors);NU1901;NU1902;NU1903;NU1904</WarningsNotAsErrors>
<!-- Warnings to always disable -->
<NoWarn>$(NoWarn);CS8500;CS8969</NoWarn>
<!-- Don't warn about redundant equality -->
<NoWarn>$(NoWarn);IDE0100</NoWarn>
<!-- Don't warn about unused parameters -->
<NoWarn>$(NoWarn);IDE0060</NoWarn>
<!-- Suppress "CS1591 - Missing XML comment for publicly visible type or member" compiler errors for private assemblies. -->
<NoWarn Condition="'$(IsPrivateAssembly)' == 'true'">$(NoWarn);CS1591</NoWarn>
<!-- Always pass portable to override arcade sdk which uses embedded for local builds -->
Expand Down
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
Source-build builds the product with the most recent previously source-built release. Thankfully, these two requirements line up nicely
such that any version that satisfies the VS version requirement will also satisfy the .NET SDK version requirement because of how we ship.
-->
<MicrosoftCodeAnalysisVersion_LatestVS>4.8.0</MicrosoftCodeAnalysisVersion_LatestVS>
<MicrosoftCodeAnalysisVersion_LatestVS>4.14.0</MicrosoftCodeAnalysisVersion_LatestVS>
<!-- Some of the analyzer dependencies used by ILLink project -->
<MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.5-beta1.23270.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,20 @@ private void VerifyLoggedMessages(AssemblyDefinition original, TrimmingTestLogge
List<(ICustomAttributeProvider, CustomAttribute)> expectedNoWarningsAttributes = new();
foreach (var attrProvider in GetAttributeProviders(original))
{
if (attrProvider is IMemberDefinition attrMember &&
attrMember is not TypeDefinition &&
attrMember.DeclaringType is TypeDefinition declaringType &&
declaringType.Name.StartsWith("<G>"))
{
// Workaround: C# 14 extension members result in a compiler-generated type
// that has a member for each extension member (this is in addition to the type
// which contains the actual extension member implementation).
// The generated members inherit attributes from the extension members, but
// have empty implementations. We don't want to check inherited ExpectedWarningAttributes
// for these members.
continue;
}

foreach (var attr in attrProvider.CustomAttributes)
{
if (!IsProducedByNativeAOT(attr))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,16 @@ bool merge
return value;
}

// Property may be an indexer, in which case there will be one or more index arguments followed by a value argument
ImmutableArray<TValue>.Builder arguments = ImmutableArray.CreateBuilder<TValue>();

// Handle C# 14 extension property access (see comment in ProcessMethodCall)
if (setMethod.HasExtensionParameterOnType())
{
arguments.Add(instanceValue);
instanceValue = TopValue;
}

// Property may be an indexer, in which case there will be one or more index arguments followed by a value argument
foreach (var val in propertyRef.Arguments)
arguments.Add(Visit(val, state));
arguments.Add(value);
Expand Down Expand Up @@ -705,14 +713,20 @@ public override TValue VisitPropertyReference(IPropertyReferenceOperation operat
// Accessing property for reading is really a call to the getter
// The setter case is handled in assignment operation since here we don't have access to the value to pass to the setter
TValue instanceValue = Visit(operation.Instance, state);
IMethodSymbol? getMethod = operation.Property.GetGetMethod();
IMethodSymbol getMethod = operation.Property.GetGetMethod()!;
ImmutableArray<TValue>.Builder arguments = ImmutableArray.CreateBuilder<TValue>();
// Handle C# 14 extension property access (see comment in ProcessMethodCall)
if (getMethod.HasExtensionParameterOnType())
{
arguments.Add(instanceValue);
instanceValue = TopValue;
}

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

return HandleMethodCallHelper(getMethod!, instanceValue, arguments.ToImmutableArray(), operation, state);
return HandleMethodCallHelper(getMethod, instanceValue, arguments.ToImmutableArray(), operation, state);
}

public override TValue VisitEventReference(IEventReferenceOperation operation, LocalDataFlowState<TValue, TContext, TValueLattice, TContextLattice> state)
Expand Down Expand Up @@ -890,7 +904,7 @@ private TValue HandleMethodCallHelper(
TConditionValue conditionValue = GetConditionValue(argumentOperation, state);
var current = state.Current;
ApplyCondition(
doesNotReturnIfConditionValue == false
!doesNotReturnIfConditionValue
? conditionValue
: conditionValue.Negate(),
ref current);
Expand All @@ -910,6 +924,16 @@ private TValue ProcessMethodCall(
TValue instanceValue = Visit(instance, state);

var argumentsBuilder = ImmutableArray.CreateBuilder<TValue>();

// For calls to C# 14 extensions, treat the instance argument as a regular argument.
// The extension method doesn't have an implicit this, yet (unlike for existing extension methods)
// the IOperation represents it as having an Instance.
if (method.HasExtensionParameterOnType())
{
argumentsBuilder.Add(instanceValue);
instanceValue = TopValue;
}

foreach (var argument in arguments)
{
// For __arglist argument there might not be any parameter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,11 @@ private static void VerifyDamOnPropertyAndAccessorMatch(SymbolAnalysisContext co
|| propertySymbol.GetDynamicallyAccessedMemberTypes() == DynamicallyAccessedMemberTypes.None)
return;

// For C# 14 extension properties, property-level DAM does not propagate to accessors
// and we do not consider property vs accessor conflicts meaningful. Skip conflict checks.
if (methodSymbol.HasExtensionParameterOnType())
return;

// None on the return type of 'get' matches unannotated
if (methodSymbol.MethodKind == MethodKind.PropertyGet
&& methodSymbol.GetDynamicallyAccessedMemberTypesOnReturnType() != DynamicallyAccessedMemberTypes.None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<NoWarn Condition="'$(DotNetBuildSourceOnly)' == 'true'">$(NoWarn);CS8524</NoWarn>
<!-- Type conflicts with imported type. Silence this for source build to allow building with the latest
source-included type name parser, instead of the one that comes with Roslyn. -->
<NoWarn Condition="'$(DotNetBuildSourceOnly)' == 'true'">$(NoWarn);CS0436</NoWarn>
<NoWarn>$(NoWarn);CS0436</NoWarn>
<AnalyzerLanguage>cs</AnalyzerLanguage>
<!-- The analyzer needs to process deeply nested expressions in corelib.
This can blow up the stack if using unoptimized code (due to large
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,23 @@ public static ParameterProxy GetParameter(this IMethodSymbol method, ParameterIn
/// </summary>
public static int GetMetadataParametersCount(this IMethodSymbol method)
{
return method.Parameters.Length;
return method.Parameters.Length + (method.HasExtensionParameterOnType() ? 1 : 0);
}

/// <summary>
/// Gets the number of parameters pushed onto the stack when the method is called (including the implicit 'this' paramter)
/// </summary>
public static int GetParametersCount(this IMethodSymbol method)
{
return method.Parameters.Length + (method.HasImplicitThis() ? 1 : 0);
return method.Parameters.Length + (method.HasImplicitThis() ? 1 : 0) + (method.HasExtensionParameterOnType() ? 1 : 0);
}

/// <summary>
/// Returns true if the method's containing type has an extension parameter
/// </summary>
public static bool HasExtensionParameterOnType(this IMethodSymbol method)
{
return method.ContainingType.ExtensionParameter != null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,17 @@ internal static DynamicallyAccessedMemberTypes GetMethodParameterAnnotation(Para

var damt = parameter.GetDynamicallyAccessedMemberTypes();

var parameterMethod = (IMethodSymbol)parameter.ContainingSymbol;
Debug.Assert(parameterMethod != null);
IMethodSymbol parameterMethod = param.Method.Method;

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

// Is this a property setter `value` parameter?
if (parameterMethod!.MethodKind == MethodKind.PropertySet
&& damt == DynamicallyAccessedMemberTypes.None
&& parameter.Ordinal == parameterMethod.Parameters.Length - 1)
&& parameter.Ordinal == parameterMethod.Parameters.Length - 1
// Do not propagate property-level DAM for C# 14 extension properties
&& !parameterMethod.HasExtensionParameterOnType())
{
var property = (IPropertySymbol)parameterMethod.AssociatedSymbol!;
Debug.Assert(property != null);
Expand All @@ -194,7 +195,10 @@ public static DynamicallyAccessedMemberTypes GetMethodReturnValueAnnotation(IMet
// Is this a property getter?
// If there are conflicts between the getter and the property annotation,
// the getter annotation wins. (But DAMT.None is ignored)
if (method.MethodKind is MethodKind.PropertyGet && returnDamt == DynamicallyAccessedMemberTypes.None)
if (method.MethodKind is MethodKind.PropertyGet
&& returnDamt == DynamicallyAccessedMemberTypes.None
// Do not propagate property-level DAM for C# 14 extension properties]
&& !method.HasExtensionParameterOnType())
{
var property = (IPropertySymbol)method.AssociatedSymbol!;
Debug.Assert(property != null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ namespace ILLink.Shared.TrimAnalysis
{
internal partial record MethodParameterValue
{
public MethodParameterValue(IParameterSymbol parameterSymbol)
: this(new ParameterProxy(parameterSymbol)) { }
public MethodParameterValue(IMethodSymbol methodSymbol, ParameterIndex parameterIndex, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes)
: this(new(new(methodSymbol), parameterIndex), dynamicallyAccessedMemberTypes) { }

public MethodParameterValue(ParameterProxy parameter)
: this(parameter, FlowAnnotations.GetMethodParameterAnnotation(parameter)) { }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ namespace ILLink.Shared.TypeSystemProxy
{
internal partial struct ParameterProxy
{
public ParameterProxy(IParameterSymbol parameter)
public ParameterProxy(IParameterSymbol parameter, IMethodSymbol method)
{
Method = new((IMethodSymbol)parameter.ContainingSymbol);
Index = (ParameterIndex)parameter.Ordinal + (Method.HasImplicitThis() ? 1 : 0);
Method = new(method);
Index = (ParameterIndex)parameter.Ordinal +
(Method.HasImplicitThis() ? 1 : 0) +
(method.HasExtensionParameterOnType() && parameter.ContainingSymbol is IMethodSymbol ? 1 : 0);
}

public partial ReferenceKind GetReferenceKind()
Expand All @@ -22,32 +24,51 @@ public partial ReferenceKind GetReferenceKind()
? ReferenceKind.Ref
: ReferenceKind.None;

switch (Method.Method.Parameters[MetadataIndex].RefKind)
switch (ParameterSymbol!.RefKind)
{
case RefKind.Ref: return ReferenceKind.Ref;
case RefKind.In: return ReferenceKind.In;
case RefKind.Out: return ReferenceKind.Out;
case RefKind.None: return ReferenceKind.None;
default:
Debug.Fail($"Unexpected RefKind {Method.Method.Parameters[MetadataIndex].RefKind} found on parameter {GetDisplayName()}");
Debug.Fail($"Unexpected RefKind {ParameterSymbol!.RefKind} found on parameter {GetDisplayName()}");
return ReferenceKind.None;
}
}

/// <summary>
/// Returns the IParameterSymbol representing the parameter. Returns null for the implicit this paramter.
/// </summary>
public IParameterSymbol? ParameterSymbol => IsImplicitThis ? null : Method.Method.Parameters[MetadataIndex];
public IParameterSymbol? ParameterSymbol
{
get
{
if (IsImplicitThis)
{
return null;
}

var method = Method.Method;
int hasExtensionParameter = 0;
if (method.HasExtensionParameterOnType())
{
if (MetadataIndex == 0)
{
return method.ContainingType.ExtensionParameter;
}
hasExtensionParameter = 1;
}

return method.Parameters[MetadataIndex - hasExtensionParameter];
}
}

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

public TypeProxy ParameterType
=> IsImplicitThis
? new TypeProxy(Method.Method.ContainingType)
: new TypeProxy(Method.Method.Parameters[MetadataIndex].Type);
public TypeProxy ParameterType => new TypeProxy(ParameterSymbol?.Type ?? Method.Method.ContainingType);

public partial string GetDisplayName()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,8 @@ public override MultiValue VisitConversion(IConversionOperation operation, State

public override MultiValue VisitParameterReference(IParameterReferenceOperation paramRef, StateValue state)
{
IParameterSymbol parameter = paramRef.Parameter;

if (parameter.ContainingSymbol is not IMethodSymbol)
{
// TODO: Extension members allows parameters to be on types, rather than methods.
// For example: `extension<T>(ref T value) { }` will enumerate `value` where
// the containing symbol is a `NonErrorNamedTypeSymbol`
return TopValue;
}

// Reading from a parameter always returns the same annotated value. We don't track modifications.
return GetParameterTargetValue(parameter);
return GetParameterTargetValue(paramRef.Parameter);
}

public override MultiValue VisitInstanceReference(IInstanceReferenceOperation instanceRef, StateValue state)
Expand All @@ -162,7 +152,7 @@ public override MultiValue VisitInstanceReference(IInstanceReferenceOperation in
// It can also happen that we see this for a static method - for example a delegate creation
// over a local function does this, even thought the "this" makes no sense inside a static scope.
if (OwningSymbol is IMethodSymbol method && !method.IsStatic)
return new MethodParameterValue(method, (ParameterIndex)0, FlowAnnotations.GetMethodParameterAnnotation(new ParameterProxy(new(method), (ParameterIndex)0)));
return new MethodParameterValue(new ParameterProxy(new(method), (ParameterIndex)0));

return TopValue;
}
Expand Down Expand Up @@ -265,7 +255,9 @@ public override MultiValue GetBackingFieldTargetValue(IPropertyReferenceOperatio


public override MultiValue GetParameterTargetValue(IParameterSymbol parameter)
=> new MethodParameterValue(parameter);
{
return new MethodParameterValue(new ParameterProxy(parameter, parameter.ContainingSymbol as IMethodSymbol ?? (IMethodSymbol)OwningSymbol));
}

public override void HandleAssignment(MultiValue source, MultiValue target, IOperation operation, in FeatureContext featureContext)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,18 @@ public Task EventDataFlow()
return RunTest();
}

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

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

[Fact]
public Task FeatureCheckDataFlow()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node)
CheckMember(node);
}

public override void VisitOperatorDeclaration(OperatorDeclarationSyntax node)
{
base.VisitOperatorDeclaration(node);
CheckMember(node);
}

public override void VisitEventDeclaration(EventDeclarationSyntax node)
{
base.VisitEventDeclaration(node);
Expand Down
Loading