Skip to content

Commit

Permalink
ILLink: give info message on duplicate members only when within a sin…
Browse files Browse the repository at this point in the history 
…gle descriptors file (dotnet#101574)

In the trimmer we rely on the order of marking to determine if a descriptors file has a duplicate member. As we transition to the dependency analysis framework, the marking order changes, and this exposed scenarios where a descriptors file is found after some items have already been marked. This caused unnecessary IL2025 warnings (Duplicate preserve in descriptor file) to be reported. Instead of using the global marking state, we will keep a per-descriptor-file set of which members are preserved and only report duplicates within that set.
  • Loading branch information
@jtschuster @matouskozak
jtschuster authored and matouskozak committed Apr 30, 2024
1 parent 569dd9a commit 4b227bb
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
27 changes: 19 additions & 8 deletions src/tools/illink/src/linker/Linker.Steps/DescriptorMarker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Text;
Expand All @@ -23,14 +24,24 @@ public class DescriptorMarker : ProcessLinkerXmlBase
static readonly string[] _accessorsAll = new string[] { "all" };
static readonly char[] _accessorsSep = new char[] { ';' };

protected readonly HashSet<object> _preservedMembers;

public DescriptorMarker (LinkContext context, Stream documentStream, string xmlDocumentLocation)
: base (context, documentStream, xmlDocumentLocation)
{
_preservedMembers = new ();
}

public DescriptorMarker (LinkContext context, Stream documentStream, EmbeddedResource resource, AssemblyDefinition resourceAssembly, string xmlDocumentLocation = "<unspecified>")
: base (context, documentStream, resource, resourceAssembly, xmlDocumentLocation)
{
_preservedMembers = new ();
}

protected void LogDuplicatePreserve(string memberName, XPathNavigator duplicatePosition)
{
var origin = GetMessageOriginForPosition (duplicatePosition);
_context.LogMessage (MessageContainer.CreateInfoMessage (origin, $"Duplicate preserve of '{memberName}'"));
}

public void Mark ()
Expand Down Expand Up @@ -147,16 +158,16 @@ protected static TypePreserve GetTypePreserve (XPathNavigator nav)

protected override void ProcessField (TypeDefinition type, FieldDefinition field, XPathNavigator nav)
{
if (_context.Annotations.IsMarked (field))
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, field.FullName);
if (!_preservedMembers.Add (field))
LogDuplicatePreserve (field.FullName, nav);

_context.Annotations.Mark (field, new DependencyInfo (DependencyKind.XmlDescriptor, _xmlDocumentLocation), GetMessageOriginForPosition (nav));
}

protected override void ProcessMethod (TypeDefinition type, MethodDefinition method, XPathNavigator nav, object? customData)
{
if (_context.Annotations.IsMarked (method))
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, method.GetDisplayName ());
if (!_preservedMembers.Add (method))
LogDuplicatePreserve (method.GetDisplayName (), nav);

_context.Annotations.MarkIndirectlyCalledMethod (method);
_context.Annotations.SetAction (method, MethodAction.Parse);
Expand Down Expand Up @@ -212,8 +223,8 @@ public static string GetMethodSignature (MethodDefinition meth, bool includeGene

protected override void ProcessEvent (TypeDefinition type, EventDefinition @event, XPathNavigator nav, object? customData)
{
if (_context.Annotations.IsMarked (@event))
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, @event.FullName);
if (!_preservedMembers.Add (@event))
LogDuplicatePreserve(@event.FullName, nav);

ProcessMethod (type, @event.AddMethod, nav, customData);
ProcessMethod (type, @event.RemoveMethod, nav, customData);
Expand All @@ -224,8 +235,8 @@ protected override void ProcessProperty (TypeDefinition type, PropertyDefinition
{
string[] accessors = fromSignature ? GetAccessors (nav) : _accessorsAll;

if (_context.Annotations.IsMarked (property))
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, property.FullName);
if (!_preservedMembers.Add (property))
LogDuplicatePreserve(property.FullName, nav);

if (Array.IndexOf (accessors, "all") >= 0) {
ProcessMethodIfNotNull (type, property.GetMethod, nav, customData);
Expand Down
5 changes: 5 additions & 0 deletions src/tools/illink/src/linker/Linker/MessageContainer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,11 @@ public static MessageContainer CreateInfoMessage (string text)
return new MessageContainer (MessageCategory.Info, text, null);
}

internal static MessageContainer CreateInfoMessage (MessageOrigin origin, string text)
{
return new MessageContainer (MessageCategory.Info, text, null, "", origin);
}

/// <summary>
/// Create a diagnostics message.
/// </summary>
Expand Down
14 changes: 10 additions & 4 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/LinkXmlErrorCases.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Mono.Linker.Tests.Cases.LinkXml
{
[SetupLinkerDescriptorFile ("LinkXmlErrorCases.xml")]
[SetupLinkerArgument ("--skip-unresolved", "true")]
[SetupLinkerArgument ("--verbose")]

[ExpectedWarning ("IL2001", "TypeWithNoFields", FileName = "LinkXmlErrorCases.xml", SourceLine = 3, SourceColumn = 6)]
[ExpectedWarning ("IL2002", "TypeWithNoMethods", FileName = "LinkXmlErrorCases.xml", SourceLine = 4, SourceColumn = 6)]
Expand All @@ -17,10 +18,15 @@ namespace Mono.Linker.Tests.Cases.LinkXml
[ExpectedWarning ("IL2017", "NonExistentProperty", "TypeWithNoProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 21, SourceColumn = 8)]
[ExpectedWarning ("IL2018", "SetOnlyProperty", "TypeWithProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 25, SourceColumn = 8)]
[ExpectedWarning ("IL2019", "GetOnlyProperty", "TypeWithProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 26, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Method", Tool.Trimmer, "", FileName = "LinkXmlErrorCases.xml", SourceLine = 39, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Event", Tool.Trimmer, "", FileName = "LinkXmlErrorCases.xml", SourceLine = 40, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Field", Tool.Trimmer, "", FileName = "LinkXmlErrorCases.xml", SourceLine = 41, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Property", Tool.Trimmer, "", FileName = "LinkXmlErrorCases.xml", SourceLine = 42, SourceColumn = 8)]
[LogContains ("Duplicate preserve of 'System.Int32 Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases/TypeWithEverything::Field'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases.TypeWithEverything.TypeWithEverything()'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases.TypeWithEverything.Method()'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'System.EventHandler Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases/TypeWithEverything::Event'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases.TypeWithEverything.Event.add'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases.TypeWithEverything.Event.remove'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'System.Int32 Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases/TypeWithEverything::Property()'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases.TypeWithEverything.Property.get'", ProducedBy = Tool.Trimmer)]
[LogContains ("Duplicate preserve of 'Mono.Linker.Tests.Cases.LinkXml.LinkXmlErrorCases.TypeWithEverything.Property.set'", ProducedBy = Tool.Trimmer)]
// NativeAOT doesn't support wildcard * and will skip usages of it, including if they would warn
// https://github.com/dotnet/runtime/issues/80466
[ExpectedWarning ("IL2100", Tool.Trimmer, "", FileName = "LinkXmlErrorCases.xml", SourceLine = 50, SourceColumn = 4)]
Expand Down

0 comments on commit 4b227bb

Please sign in to comment.