diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index fd8094bc70283..e7682e267b344 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -706,7 +706,7 @@ private static bool ScanMethodBodyForFieldAccess(MethodIL body, bool write, out } } - internal void ValidateMethodAnnotationsAreSame(MethodDesc method, MethodDesc baseMethod) + internal void ValidateMethodAnnotationsAreSame(MethodDesc method, MethodDesc baseMethod, TypeSystemEntity origin) { method = method.GetTypicalMethodDefinition(); baseMethod = baseMethod.GetTypicalMethodDefinition(); @@ -715,14 +715,14 @@ internal void ValidateMethodAnnotationsAreSame(MethodDesc method, MethodDesc bas GetAnnotations(baseMethod.OwningType).TryGetAnnotation(baseMethod, out var baseMethodAnnotations); if (methodAnnotations.ReturnParameterAnnotation != baseMethodAnnotations.ReturnParameterAnnotation) - LogValidationWarning(method.Signature.ReturnType, baseMethod, method); + LogValidationWarning((method.Signature.ReturnType, method), baseMethod, origin); if (methodAnnotations.ParameterAnnotations != null || baseMethodAnnotations.ParameterAnnotations != null) { if (methodAnnotations.ParameterAnnotations == null) - ValidateMethodParametersHaveNoAnnotations(baseMethodAnnotations.ParameterAnnotations!, method, baseMethod, method); + ValidateMethodParametersHaveNoAnnotations(baseMethodAnnotations.ParameterAnnotations!, method, baseMethod, origin); else if (baseMethodAnnotations.ParameterAnnotations == null) - ValidateMethodParametersHaveNoAnnotations(methodAnnotations.ParameterAnnotations, method, baseMethod, method); + ValidateMethodParametersHaveNoAnnotations(methodAnnotations.ParameterAnnotations, method, baseMethod, origin); else { if (methodAnnotations.ParameterAnnotations.Length != baseMethodAnnotations.ParameterAnnotations.Length) @@ -734,7 +734,7 @@ internal void ValidateMethodAnnotationsAreSame(MethodDesc method, MethodDesc bas LogValidationWarning( (new MethodProxy(method)).GetParameter((ParameterIndex)parameterIndex), (new MethodProxy(baseMethod)).GetParameter((ParameterIndex)parameterIndex), - method); + origin); } } } @@ -742,9 +742,9 @@ internal void ValidateMethodAnnotationsAreSame(MethodDesc method, MethodDesc bas if (methodAnnotations.GenericParameterAnnotations != null || baseMethodAnnotations.GenericParameterAnnotations != null) { if (methodAnnotations.GenericParameterAnnotations == null) - ValidateMethodGenericParametersHaveNoAnnotations(baseMethodAnnotations.GenericParameterAnnotations!, method, baseMethod, method); + ValidateMethodGenericParametersHaveNoAnnotations(baseMethodAnnotations.GenericParameterAnnotations!, method, baseMethod, origin); else if (baseMethodAnnotations.GenericParameterAnnotations == null) - ValidateMethodGenericParametersHaveNoAnnotations(methodAnnotations.GenericParameterAnnotations, method, baseMethod, method); + ValidateMethodGenericParametersHaveNoAnnotations(methodAnnotations.GenericParameterAnnotations, method, baseMethod, origin); else { if (methodAnnotations.GenericParameterAnnotations.Length != baseMethodAnnotations.GenericParameterAnnotations.Length) @@ -757,14 +757,14 @@ internal void ValidateMethodAnnotationsAreSame(MethodDesc method, MethodDesc bas LogValidationWarning( method.Instantiation[genericParameterIndex], baseMethod.Instantiation[genericParameterIndex], - method); + origin); } } } } } - private void ValidateMethodParametersHaveNoAnnotations(DynamicallyAccessedMemberTypes[] parameterAnnotations, MethodDesc method, MethodDesc baseMethod, MethodDesc origin) + private void ValidateMethodParametersHaveNoAnnotations(DynamicallyAccessedMemberTypes[] parameterAnnotations, MethodDesc method, MethodDesc baseMethod, TypeSystemEntity origin) { for (int parameterIndex = 0; parameterIndex < parameterAnnotations.Length; parameterIndex++) { @@ -777,7 +777,7 @@ private void ValidateMethodParametersHaveNoAnnotations(DynamicallyAccessedMember } } - private void ValidateMethodGenericParametersHaveNoAnnotations(DynamicallyAccessedMemberTypes[] genericParameterAnnotations, MethodDesc method, MethodDesc baseMethod, MethodDesc origin) + private void ValidateMethodGenericParametersHaveNoAnnotations(DynamicallyAccessedMemberTypes[] genericParameterAnnotations, MethodDesc method, MethodDesc baseMethod, TypeSystemEntity origin) { for (int genericParameterIndex = 0; genericParameterIndex < genericParameterAnnotations.Length; genericParameterIndex++) { @@ -791,7 +791,7 @@ private void ValidateMethodGenericParametersHaveNoAnnotations(DynamicallyAccesse } } - private void LogValidationWarning(object provider, object baseProvider, MethodDesc origin) + private void LogValidationWarning(object provider, object baseProvider, TypeSystemEntity origin) { switch (provider) { @@ -810,9 +810,9 @@ private void LogValidationWarning(object provider, object baseProvider, MethodDe genericParameterOverride.Name, DiagnosticUtilities.GetGenericParameterDeclaringMemberDisplayName(genericParameterOverride), ((GenericParameterDesc)baseProvider).Name, DiagnosticUtilities.GetGenericParameterDeclaringMemberDisplayName((GenericParameterDesc)baseProvider)); break; - case TypeDesc: + case (TypeDesc, MethodDesc method): _logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodReturnValueBetweenOverrides, - DiagnosticUtilities.GetMethodSignatureDisplayName(origin), DiagnosticUtilities.GetMethodSignatureDisplayName((MethodDesc)baseProvider)); + DiagnosticUtilities.GetMethodSignatureDisplayName(method), DiagnosticUtilities.GetMethodSignatureDisplayName((MethodDesc)baseProvider)); break; // No fields - it's not possible to have a virtual field and override it default: diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs index 10bac5be472fe..8b8f34ada6c83 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -534,7 +534,8 @@ public sealed override IEnumerable GetConditionalSt result.Add(new CombinedDependencyListEntry(factory.VirtualMethodUse(interfaceMethod), factory.VariantInterfaceMethodUse(typicalInterfaceMethod), "Interface method")); } - factory.MetadataManager.NoteOverridingMethod(interfaceMethod, implMethod); + TypeSystemEntity origin = (implMethod.OwningType != defType) ? defType : null; + factory.MetadataManager.NoteOverridingMethod(interfaceMethod, implMethod, origin); factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, interfaceMethod, implMethod); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GVMDependenciesNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GVMDependenciesNode.cs index 26526cc5628d7..618d7e68c9b7b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GVMDependenciesNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GVMDependenciesNode.cs @@ -149,7 +149,8 @@ public override IEnumerable SearchDynamicDependenci else dynamicDependencies.Add(new CombinedDependencyListEntry(factory.GVMDependencies(implementingMethodInstantiation.GetCanonMethodTarget(CanonicalFormKind.Specific)), null, "ImplementingMethodInstantiation")); - factory.MetadataManager.NoteOverridingMethod(_method, implementingMethodInstantiation); + TypeSystemEntity origin = (implementingMethodInstantiation.OwningType != potentialOverrideType) ? potentialOverrideType : null; + factory.MetadataManager.NoteOverridingMethod(_method, implementingMethodInstantiation, origin); } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index 2aaa93c9efdcc..835b601cf6496 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -1186,7 +1186,7 @@ public virtual DependencyList GetDependenciesForCustomAttribute(NodeFactory fact return null; } - public virtual void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc overridingMethod) + public virtual void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc overridingMethod, TypeSystemEntity origin = null) { } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index 991fe7aa62401..7748b5969ce6d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -784,7 +784,7 @@ public bool GeneratesAttributeMetadata(TypeDesc attributeType) return true; } - public override void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc overridingMethod) + public override void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc overridingMethod, TypeSystemEntity origin) { baseMethod = baseMethod.GetTypicalMethodDefinition(); overridingMethod = overridingMethod.GetTypicalMethodDefinition(); @@ -792,6 +792,8 @@ public override void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc over if (baseMethod == overridingMethod) return; + origin ??= overridingMethod; + bool baseMethodTypeIsInterface = baseMethod.OwningType.IsInterface; foreach (var requiresAttribute in _requiresAttributeMismatchNameAndId) { @@ -803,7 +805,7 @@ public override void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc over string message = MessageFormat.FormatRequiresAttributeMismatch(overridingMethod.DoesMethodRequire(requiresAttribute.AttributeName, out _), baseMethodTypeIsInterface, requiresAttribute.AttributeName, overridingMethodName, baseMethodName); - Logger.LogWarning(overridingMethod, requiresAttribute.Id, message); + Logger.LogWarning(origin, requiresAttribute.Id, message); } } @@ -811,7 +813,7 @@ public override void NoteOverridingMethod(MethodDesc baseMethod, MethodDesc over bool overridingMethodRequiresDataflow = FlowAnnotations.RequiresVirtualMethodDataflowAnalysis(overridingMethod); if (baseMethodRequiresDataflow || overridingMethodRequiresDataflow) { - FlowAnnotations.ValidateMethodAnnotationsAreSame(overridingMethod, baseMethod); + FlowAnnotations.ValidateMethodAnnotationsAreSame(overridingMethod, baseMethod, origin); } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs index c7f7eb2eb6aa0..072433e7925d9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs @@ -75,7 +75,12 @@ void AddRange (DiagnosticId first, DiagnosticId last) public override ImmutableArray SupportedDiagnostics => GetSupportedDiagnostics (); - static Location GetPrimaryLocation (ImmutableArray locations) => locations.Length > 0 ? locations[0] : Location.None; + static Location GetPrimaryLocation (ImmutableArray? locations) { + if (locations is null) + return Location.None; + + return locations.Value.Length > 0 ? locations.Value[0] : Location.None; + } public override void Initialize (AnalysisContext context) { @@ -167,7 +172,7 @@ static void VerifyDamOnDerivedAndBaseMethodsMatch (SymbolAnalysisContext context } } - static void VerifyDamOnMethodsMatch (SymbolAnalysisContext context, IMethodSymbol overrideMethod, IMethodSymbol baseMethod) + static void VerifyDamOnMethodsMatch (SymbolAnalysisContext context, IMethodSymbol overrideMethod, IMethodSymbol baseMethod, ISymbol? origin = null) { var overrideMethodReturnAnnotation = FlowAnnotations.GetMethodReturnValueAnnotation (overrideMethod); var baseMethodReturnAnnotation = FlowAnnotations.GetMethodReturnValueAnnotation (baseMethod); @@ -184,9 +189,10 @@ static void VerifyDamOnMethodsMatch (SymbolAnalysisContext context, IMethodSymbo && baseMethod.TryGetReturnAttribute (DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _)) ) ? (null, null) : CreateArguments (attributableSymbolLocation, missingAttribute); + var returnOrigin = origin ??= overrideMethod; context.ReportDiagnostic (Diagnostic.Create ( DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodReturnValueBetweenOverrides), - GetPrimaryLocation (overrideMethod.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary (), overrideMethod.GetDisplayName (), baseMethod.GetDisplayName ())); + GetPrimaryLocation (returnOrigin.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary (), overrideMethod.GetDisplayName (), baseMethod.GetDisplayName ())); } foreach (var overrideParam in overrideMethod.GetMetadataParameters ()) { @@ -205,9 +211,10 @@ static void VerifyDamOnMethodsMatch (SymbolAnalysisContext context, IMethodSymbo && baseParam.ParameterSymbol!.TryGetAttribute (DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _)) ) ? (null, null) : CreateArguments (attributableSymbolLocation, missingAttribute); + var parameterOrigin = origin ?? overrideParam.ParameterSymbol; context.ReportDiagnostic (Diagnostic.Create ( DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodParameterBetweenOverrides), - overrideParam.Location, sourceLocation, DAMArgs?.ToImmutableDictionary (), + GetPrimaryLocation (parameterOrigin?.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary (), overrideParam.GetDisplayName (), overrideMethod.GetDisplayName (), baseParam.GetDisplayName (), baseMethod.GetDisplayName ())); } } @@ -228,27 +235,38 @@ static void VerifyDamOnMethodsMatch (SymbolAnalysisContext context, IMethodSymbo && baseMethod.TypeParameters[i].TryGetAttribute (DynamicallyAccessedMembersAnalyzer.DynamicallyAccessedMembersAttribute, out var _)) ) ? (null, null) : CreateArguments (attributableSymbolLocation, missingAttribute); + var typeParameterOrigin = origin ?? overrideMethod.TypeParameters[i]; context.ReportDiagnostic (Diagnostic.Create ( DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersMismatchOnGenericParameterBetweenOverrides), - GetPrimaryLocation (overrideMethod.TypeParameters[i].Locations), sourceLocation, DAMArgs?.ToImmutableDictionary (), + GetPrimaryLocation (typeParameterOrigin.Locations), sourceLocation, DAMArgs?.ToImmutableDictionary (), overrideMethod.TypeParameters[i].GetDisplayName (), overrideMethod.GetDisplayName (), baseMethod.TypeParameters[i].GetDisplayName (), baseMethod.GetDisplayName ())); } } - if (!overrideMethod.IsStatic && overrideMethod.GetDynamicallyAccessedMemberTypes () != baseMethod.GetDynamicallyAccessedMemberTypes ()) + if (!overrideMethod.IsStatic && overrideMethod.GetDynamicallyAccessedMemberTypes () != baseMethod.GetDynamicallyAccessedMemberTypes ()) { + var methodOrigin = origin ?? overrideMethod; context.ReportDiagnostic (Diagnostic.Create ( DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.DynamicallyAccessedMembersMismatchOnImplicitThisBetweenOverrides), - GetPrimaryLocation (overrideMethod.Locations), + GetPrimaryLocation (methodOrigin.Locations), overrideMethod.GetDisplayName (), baseMethod.GetDisplayName ())); + } } static void VerifyDamOnInterfaceAndImplementationMethodsMatch (SymbolAnalysisContext context, INamedTypeSymbol type) { foreach (var (interfaceMember, implementationMember) in type.GetMemberInterfaceImplementationPairs ()) { - if (implementationMember is IMethodSymbol implementationMethod - && interfaceMember is IMethodSymbol interfaceMethod) - VerifyDamOnMethodsMatch (context, implementationMethod, interfaceMethod); + if (implementationMember is IMethodSymbol implementationMethod && interfaceMember is IMethodSymbol interfaceMethod) { + ISymbol origin = implementationMethod; + INamedTypeSymbol implementationType = implementationMethod.ContainingType; + + // If this type implements an interface method through a base class, the origin of the warning is this type, + // not the member on the base class. + if (!implementationType.IsInterface () && !SymbolEqualityComparer.Default.Equals (implementationType, type)) + origin = type; + + VerifyDamOnMethodsMatch (context, implementationMethod, interfaceMethod, origin); + } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index 28118233b656a..b104fc7dd8ead 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -123,8 +123,21 @@ void CheckMatchingAttributesInInterfaces ( INamedTypeSymbol type) { foreach (var memberpair in type.GetMemberInterfaceImplementationPairs ()) { + var implementationType = memberpair.ImplementationMember switch { + IMethodSymbol method => method.ContainingType, + IPropertySymbol property => property.ContainingType, + IEventSymbol @event => @event.ContainingType, + _ => throw new NotSupportedException () + }; + ISymbol origin = memberpair.ImplementationMember; + + // If this type implements an interface method through a base class, the origin of the warning is this type, + // not the member on the base class. + if (!implementationType.IsInterface () && !SymbolEqualityComparer.Default.Equals (implementationType, type)) + origin = type; + if (HasMismatchingAttributes (memberpair.InterfaceMember, memberpair.ImplementationMember)) { - ReportMismatchInAttributesDiagnostic (symbolAnalysisContext, memberpair.ImplementationMember, memberpair.InterfaceMember, isInterface: true); + ReportMismatchInAttributesDiagnostic (symbolAnalysisContext, memberpair.ImplementationMember, memberpair.InterfaceMember, isInterface: true, origin); } } } @@ -230,12 +243,13 @@ private void ReportRequiresOnStaticCtorDiagnostic (SymbolAnalysisContext symbolA ctor.GetDisplayName ())); } - private void ReportMismatchInAttributesDiagnostic (SymbolAnalysisContext symbolAnalysisContext, ISymbol member, ISymbol baseMember, bool isInterface = false) + private void ReportMismatchInAttributesDiagnostic (SymbolAnalysisContext symbolAnalysisContext, ISymbol member, ISymbol baseMember, bool isInterface = false, ISymbol? origin = null) { + origin ??= member; string message = MessageFormat.FormatRequiresAttributeMismatch (member.HasAttribute (RequiresAttributeName), isInterface, RequiresAttributeName, member.GetDisplayName (), baseMember.GetDisplayName ()); symbolAnalysisContext.ReportDiagnostic (Diagnostic.Create ( RequiresAttributeMismatch, - member.Locations[0], + origin.Locations[0], message)); } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 3b148cf3a1ac5..14617b0de9ca1 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -461,19 +461,21 @@ bool ScanMethodBodyForFieldAccess (MethodBody body, bool write, out FieldDefinit return true; } - internal void ValidateMethodAnnotationsAreSame (MethodDefinition method, MethodDefinition baseMethod) + internal void ValidateMethodAnnotationsAreSame (OverrideInformation ov) { + var method = ov.Override; + var baseMethod = ov.Base; GetAnnotations (method.DeclaringType).TryGetAnnotation (method, out var methodAnnotations); GetAnnotations (baseMethod.DeclaringType).TryGetAnnotation (baseMethod, out var baseMethodAnnotations); if (methodAnnotations.ReturnParameterAnnotation != baseMethodAnnotations.ReturnParameterAnnotation) - LogValidationWarning (method.MethodReturnType, baseMethod.MethodReturnType, method); + LogValidationWarning (method.MethodReturnType, baseMethod.MethodReturnType, ov); if (methodAnnotations.ParameterAnnotations != null || baseMethodAnnotations.ParameterAnnotations != null) { if (methodAnnotations.ParameterAnnotations == null) - ValidateMethodParametersHaveNoAnnotations (baseMethodAnnotations.ParameterAnnotations!, method, baseMethod, method); + ValidateMethodParametersHaveNoAnnotations (baseMethodAnnotations.ParameterAnnotations!, ov); else if (baseMethodAnnotations.ParameterAnnotations == null) - ValidateMethodParametersHaveNoAnnotations (methodAnnotations.ParameterAnnotations, method, baseMethod, method); + ValidateMethodParametersHaveNoAnnotations (methodAnnotations.ParameterAnnotations, ov); else { if (methodAnnotations.ParameterAnnotations.Length != baseMethodAnnotations.ParameterAnnotations.Length) return; @@ -483,16 +485,16 @@ internal void ValidateMethodAnnotationsAreSame (MethodDefinition method, MethodD LogValidationWarning ( method.TryGetParameter ((ParameterIndex) parameterIndex)?.GetCustomAttributeProvider ()!, baseMethod.TryGetParameter ((ParameterIndex) parameterIndex)?.GetCustomAttributeProvider ()!, - method); + ov); } } } if (methodAnnotations.GenericParameterAnnotations != null || baseMethodAnnotations.GenericParameterAnnotations != null) { if (methodAnnotations.GenericParameterAnnotations == null) - ValidateMethodGenericParametersHaveNoAnnotations (baseMethodAnnotations.GenericParameterAnnotations!, method, baseMethod, method); + ValidateMethodGenericParametersHaveNoAnnotations (baseMethodAnnotations.GenericParameterAnnotations!, ov); else if (baseMethodAnnotations.GenericParameterAnnotations == null) - ValidateMethodGenericParametersHaveNoAnnotations (methodAnnotations.GenericParameterAnnotations, method, baseMethod, method); + ValidateMethodGenericParametersHaveNoAnnotations (methodAnnotations.GenericParameterAnnotations, ov); else { if (methodAnnotations.GenericParameterAnnotations.Length != baseMethodAnnotations.GenericParameterAnnotations.Length) return; @@ -502,39 +504,42 @@ internal void ValidateMethodAnnotationsAreSame (MethodDefinition method, MethodD LogValidationWarning ( method.GenericParameters[genericParameterIndex], baseMethod.GenericParameters[genericParameterIndex], - method); + ov); } } } } } - void ValidateMethodParametersHaveNoAnnotations (DynamicallyAccessedMemberTypes[] parameterAnnotations, MethodDefinition method, MethodDefinition baseMethod, IMemberDefinition origin) + void ValidateMethodParametersHaveNoAnnotations (DynamicallyAccessedMemberTypes[] parameterAnnotations, OverrideInformation ov) { for (int parameterIndex = 0; parameterIndex < parameterAnnotations.Length; parameterIndex++) { var annotation = parameterAnnotations[parameterIndex]; if (annotation != DynamicallyAccessedMemberTypes.None) LogValidationWarning ( - method.GetParameter ((ParameterIndex) parameterIndex).GetCustomAttributeProvider ()!, - baseMethod.GetParameter ((ParameterIndex) parameterIndex).GetCustomAttributeProvider ()!, - origin); + ov.Override.GetParameter ((ParameterIndex) parameterIndex).GetCustomAttributeProvider ()!, + ov.Base.GetParameter ((ParameterIndex) parameterIndex).GetCustomAttributeProvider ()!, + ov); } } - void ValidateMethodGenericParametersHaveNoAnnotations (DynamicallyAccessedMemberTypes[] genericParameterAnnotations, MethodDefinition method, MethodDefinition baseMethod, IMemberDefinition origin) + void ValidateMethodGenericParametersHaveNoAnnotations (DynamicallyAccessedMemberTypes[] genericParameterAnnotations, OverrideInformation ov) { for (int genericParameterIndex = 0; genericParameterIndex < genericParameterAnnotations.Length; genericParameterIndex++) { if (genericParameterAnnotations[genericParameterIndex] != DynamicallyAccessedMemberTypes.None) { LogValidationWarning ( - method.GenericParameters[genericParameterIndex], - baseMethod.GenericParameters[genericParameterIndex], - origin); + ov.Override.GenericParameters[genericParameterIndex], + ov.Base.GenericParameters[genericParameterIndex], + ov); } } } - void LogValidationWarning (IMetadataTokenProvider provider, IMetadataTokenProvider baseProvider, IMemberDefinition origin) + void LogValidationWarning (IMetadataTokenProvider provider, IMetadataTokenProvider baseProvider, OverrideInformation ov) { + IMemberDefinition origin = (ov.IsOverrideOfInterfaceMember && ov.InterfaceImplementor.Implementor != ov.Override.DeclaringType) + ? ov.InterfaceImplementor.Implementor + : ov.Override; Debug.Assert (provider.GetType () == baseProvider.GetType ()); Debug.Assert (!(provider is GenericParameter genericParameter) || genericParameter.DeclaringMethod != null); switch (provider) { diff --git a/src/tools/illink/src/linker/Linker.Steps/ValidateVirtualMethodAnnotationsStep.cs b/src/tools/illink/src/linker/Linker.Steps/ValidateVirtualMethodAnnotationsStep.cs index ac4b07e6681b4..aa9c7ee0d9f09 100644 --- a/src/tools/illink/src/linker/Linker.Steps/ValidateVirtualMethodAnnotationsStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/ValidateVirtualMethodAnnotationsStep.cs @@ -16,8 +16,8 @@ protected override void Process () var baseOverrideInformations = annotations.GetBaseMethods (method); if (baseOverrideInformations != null) { foreach (var baseOv in baseOverrideInformations) { - annotations.FlowAnnotations.ValidateMethodAnnotationsAreSame (method, baseOv.Base); - ValidateMethodRequiresUnreferencedCodeAreSame (method, baseOv.Base); + annotations.FlowAnnotations.ValidateMethodAnnotationsAreSame (baseOv); + ValidateMethodRequiresUnreferencedCodeAreSame (baseOv); } } @@ -30,15 +30,17 @@ protected override void Process () if (annotations.VirtualMethodsWithAnnotationsToValidate.Contains (overrideInformation.Override)) continue; - annotations.FlowAnnotations.ValidateMethodAnnotationsAreSame (overrideInformation.Override, method); - ValidateMethodRequiresUnreferencedCodeAreSame (overrideInformation.Override, method); + annotations.FlowAnnotations.ValidateMethodAnnotationsAreSame (overrideInformation); + ValidateMethodRequiresUnreferencedCodeAreSame (overrideInformation); } } } } - void ValidateMethodRequiresUnreferencedCodeAreSame (MethodDefinition method, MethodDefinition baseMethod) + void ValidateMethodRequiresUnreferencedCodeAreSame (OverrideInformation ov) { + var method = ov.Override; + var baseMethod = ov.Base; var annotations = Context.Annotations; bool methodSatisfies = annotations.IsInRequiresUnreferencedCodeScope (method, out _); bool baseRequires = annotations.DoesMethodRequireUnreferencedCode (baseMethod, out _); @@ -49,7 +51,10 @@ void ValidateMethodRequiresUnreferencedCodeAreSame (MethodDefinition method, Met nameof (RequiresUnreferencedCodeAttribute), method.GetDisplayName (), baseMethod.GetDisplayName ()); - Context.LogWarning (method, DiagnosticId.RequiresUnreferencedCodeAttributeMismatch, message); + IMemberDefinition origin = (ov.IsOverrideOfInterfaceMember && ov.InterfaceImplementor.Implementor != method.DeclaringType) + ? ov.InterfaceImplementor.Implementor + : method; + Context.LogWarning (origin, DiagnosticId.RequiresUnreferencedCodeAttributeMismatch, message); } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InterfaceImplementedThroughBaseValidation.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InterfaceImplementedThroughBaseValidation.cs new file mode 100644 index 0000000000000..b92d3fa3943f2 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/InterfaceImplementedThroughBaseValidation.cs @@ -0,0 +1,339 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Diagnostics.CodeAnalysis; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.DataFlow +{ + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + class InterfaceImplementedThroughBaseValidation + { + public static void Main () + { + RUCOnInterfaceMethod.Test (); + RUCOnBaseMethod.Test (); + DAMOnInterfaceMethod.Test (); + DAMOnBaseMethod.Test (); + + RUCOnInterfaceWithDIM.Test (); + RUCOnDIM.Test (); + DAMOnInterfaceWithDIM.Test (); + DAMOnDIM.Test (); + + GenericVirtualMethod.Test (); + GenericVirtualMethodWithDIM.Test (); + GenericVirtualMethodWithStaticDIM.Test (); + + CalledThroughConstraint.Test (); + CalledThroughConstraintWithDIM.Test (); + } + + class RUCOnInterfaceMethod + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + void Method (); + } + + class Base { + public void Method () {} + } + + [ExpectedWarning ("IL2046")] + class Derived : Base, Interface {} + + [ExpectedWarning ("IL2026")] + public static void Test () { + Interface i = new Derived (); + i.Method (); + } + } + + class RUCOnBaseMethod + { + interface Interface { + void Method (); + } + + class Base { + [RequiresUnreferencedCode (nameof (Method))] + public void Method () {} + } + + [ExpectedWarning ("IL2046")] + class Derived : Base, Interface {} + + public static void Test () { + Interface i = new Derived (); + i.Method (); + } + } + + class DAMOnInterfaceMethod + { + interface Interface { + void Method ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t); + } + + class Base { + public void Method (Type t) {} + } + + [ExpectedWarning ("IL2092")] + class Derived : Base, Interface {} + + public static void Test () { + Interface i = new Derived (); + i.Method (typeof (int)); + } + } + + class DAMOnBaseMethod + { + interface Interface { + void Method (Type t); + } + + class Base { + public void Method ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) {} + } + + [ExpectedWarning ("IL2092")] + class Derived : Base, Interface {} + + public static void Test () { + Interface i = new Derived (); + i.Method (typeof (int)); + } + } + + class RUCOnInterfaceWithDIM + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + void Method (); + } + + interface InterfaceImpl : Interface { + [ExpectedWarning ("IL2046")] + [ExpectedWarning ("IL2046", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104627")] + [ExpectedWarning ("IL2046", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/104627")] + void Interface.Method() {} + } + + class C : InterfaceImpl {} + + class D : InterfaceImpl {} + + [ExpectedWarning ("IL2026")] + public static void Test () { + Interface i = new C (); + i = new D (); + i.Method (); + } + } + + class RUCOnDIM + { + interface Interface { + void Method (); + } + + interface InterfaceImpl : Interface { + [ExpectedWarning ("IL2046")] + [ExpectedWarning ("IL2046", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104627")] + [ExpectedWarning ("IL2046", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/104627")] + [RequiresUnreferencedCode (nameof (Method))] + void Interface.Method() {} + } + + class C : InterfaceImpl {} + + class D : InterfaceImpl {} + + public static void Test () { + Interface i = new C (); + i = new D (); + i.Method (); + } + } + + class DAMOnInterfaceWithDIM + { + interface Interface { + void Method ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t); + } + + interface InterfaceImpl : Interface { + [ExpectedWarning ("IL2092")] + [ExpectedWarning ("IL2092", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104627")] + [ExpectedWarning ("IL2092", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/104627")] + void Interface.Method (Type t) {} + } + + class C : InterfaceImpl {} + + class D : InterfaceImpl {} + + public static void Test () { + Interface i = new C (); + i = new D (); + i.Method (typeof (int)); + } + } + + class DAMOnDIM + { + interface Interface { + void Method (Type t); + } + + interface InterfaceImpl : Interface { + [ExpectedWarning ("IL2092")] + [ExpectedWarning ("IL2092", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104627")] + [ExpectedWarning ("IL2092", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/104627")] + void Interface.Method ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type t) {} + } + + class C : InterfaceImpl {} + + class D : InterfaceImpl {} + + public static void Test () { + Interface i = new C (); + i = new D (); + i.Method (typeof (int)); + } + } + + class CalledThroughConstraint + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + void Method (); + } + + struct Struct : Interface { + [ExpectedWarning ("IL2046")] + [ExpectedWarning ("IL2046", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104627")] + public void Method () {} + } + + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL2026")] + static void Call (T t) where T : Interface { + t.Method (); + t.Method (); + } + + public static void Test () { + Call(new Struct ()); + } + } + + class CalledThroughConstraintWithDIM + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + static abstract void Method (); + } + + interface InterfaceImpl : Interface { + [ExpectedWarning ("IL2046")] + [ExpectedWarning ("IL2046", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104702")] + static void Interface.Method() {} + } + + struct Struct : InterfaceImpl {} + + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL2026")] + static void Call () where T : Interface { + T.Method (); + T.Method (); + } + + public static void Test () { + Call (); + } + } + + class GenericVirtualMethod + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + void Method (); + } + + class Base { + public void Method () {} + } + + [ExpectedWarning ("IL2046")] + class Derived : Base, Interface {} + + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL2026")] + public static void Test () { + Interface i = new Derived (); + i.Method (); + i.Method (); + } + } + + class GenericVirtualMethodWithDIM + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + void Method (); + } + + class Impl : Interface { + [ExpectedWarning ("IL2046")] + void Interface.Method () {} + } + + class Class : Impl {} + + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL2026")] + public static void Test () { + Interface i = new Class (); + i.Method (); + i.Method (); + } + } + + class GenericVirtualMethodWithStaticDIM + { + interface Interface { + [RequiresUnreferencedCode (nameof (Method))] + static abstract void Method (); + } + + interface Impl : Interface { + [ExpectedWarning ("IL2046")] + [ExpectedWarning ("IL2046", Tool.Analyzer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/104702")] + static void Interface.Method () {} + } + + class Class : Impl {} + + [ExpectedWarning ("IL2026")] + [ExpectedWarning ("IL2026")] + static void Call () where T : Interface { + T.Method (); + T.Method (); + } + + public static void Test () { + Call (); + } + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/VirtualMethodHierarchyDataflowAnnotationValidation.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/VirtualMethodHierarchyDataflowAnnotationValidation.cs index d19bcfe349aac..ea71c1f250abf 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/VirtualMethodHierarchyDataflowAnnotationValidation.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/VirtualMethodHierarchyDataflowAnnotationValidation.cs @@ -574,17 +574,17 @@ interface IBaseImplementedInterface class BaseImplementsInterfaceViaDerived { - [LogContains ( - "'DynamicallyAccessedMemberTypes' in 'DynamicallyAccessedMembersAttribute' on the return value of method 'Mono.Linker.Tests.Cases.DataFlow.VirtualMethodHierarchyDataflowAnnotationValidation.BaseImplementsInterfaceViaDerived.ReturnValueBaseWithInterfaceWithout()' " + - "don't match overridden return value of method 'Mono.Linker.Tests.Cases.DataFlow.VirtualMethodHierarchyDataflowAnnotationValidation.IBaseImplementedInterface.ReturnValueBaseWithInterfaceWithout()'. " + - "All overridden members must have the same 'DynamicallyAccessedMembersAttribute' usage.")] [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] public virtual Type ReturnValueBaseWithInterfaceWithout () => null; - [ExpectedWarning ("IL2046", "BaseImplementsInterfaceViaDerived.RequiresUnreferencedCodeBaseWithoutInterfaceWith")] public virtual void RequiresUnreferencedCodeBaseWithoutInterfaceWith () { } } + [ExpectedWarning ("IL2046", "BaseImplementsInterfaceViaDerived.RequiresUnreferencedCodeBaseWithoutInterfaceWith")] + [LogContains ( + "'DynamicallyAccessedMemberTypes' in 'DynamicallyAccessedMembersAttribute' on the return value of method 'Mono.Linker.Tests.Cases.DataFlow.VirtualMethodHierarchyDataflowAnnotationValidation.BaseImplementsInterfaceViaDerived.ReturnValueBaseWithInterfaceWithout()' " + + "don't match overridden return value of method 'Mono.Linker.Tests.Cases.DataFlow.VirtualMethodHierarchyDataflowAnnotationValidation.IBaseImplementedInterface.ReturnValueBaseWithInterfaceWithout()'. " + + "All overridden members must have the same 'DynamicallyAccessedMembersAttribute' usage.")] class DerivedWithInterfaceImplementedByBase : BaseImplementsInterfaceViaDerived, IBaseImplementedInterface { }