Skip to content

Commit c6883c1

Browse files
jtschustermichaelgsharp
authored andcommitted
ILLink: give info message on duplicate members only when within a single 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.
1 parent 0bdc91e commit c6883c1

File tree

3 files changed

+34
-12
lines changed

3 files changed

+34
-12
lines changed

src/tools/illink/src/linker/Linker.Steps/DescriptorMarker.cs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System;
5+
using System.Collections.Generic;
56
using System.Diagnostics;
67
using System.IO;
78
using System.Text;
@@ -23,14 +24,24 @@ public class DescriptorMarker : ProcessLinkerXmlBase
2324
static readonly string[] _accessorsAll = new string[] { "all" };
2425
static readonly char[] _accessorsSep = new char[] { ';' };
2526

27+
protected readonly HashSet<object> _preservedMembers;
28+
2629
public DescriptorMarker (LinkContext context, Stream documentStream, string xmlDocumentLocation)
2730
: base (context, documentStream, xmlDocumentLocation)
2831
{
32+
_preservedMembers = new ();
2933
}
3034

3135
public DescriptorMarker (LinkContext context, Stream documentStream, EmbeddedResource resource, AssemblyDefinition resourceAssembly, string xmlDocumentLocation = "<unspecified>")
3236
: base (context, documentStream, resource, resourceAssembly, xmlDocumentLocation)
3337
{
38+
_preservedMembers = new ();
39+
}
40+
41+
protected void LogDuplicatePreserve(string memberName, XPathNavigator duplicatePosition)
42+
{
43+
var origin = GetMessageOriginForPosition (duplicatePosition);
44+
_context.LogMessage (MessageContainer.CreateInfoMessage (origin, $"Duplicate preserve of '{memberName}'"));
3445
}
3546

3647
public void Mark ()
@@ -147,16 +158,16 @@ protected static TypePreserve GetTypePreserve (XPathNavigator nav)
147158

148159
protected override void ProcessField (TypeDefinition type, FieldDefinition field, XPathNavigator nav)
149160
{
150-
if (_context.Annotations.IsMarked (field))
151-
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, field.FullName);
161+
if (!_preservedMembers.Add (field))
162+
LogDuplicatePreserve (field.FullName, nav);
152163

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

156167
protected override void ProcessMethod (TypeDefinition type, MethodDefinition method, XPathNavigator nav, object? customData)
157168
{
158-
if (_context.Annotations.IsMarked (method))
159-
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, method.GetDisplayName ());
169+
if (!_preservedMembers.Add (method))
170+
LogDuplicatePreserve (method.GetDisplayName (), nav);
160171

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

213224
protected override void ProcessEvent (TypeDefinition type, EventDefinition @event, XPathNavigator nav, object? customData)
214225
{
215-
if (_context.Annotations.IsMarked (@event))
216-
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, @event.FullName);
226+
if (!_preservedMembers.Add (@event))
227+
LogDuplicatePreserve(@event.FullName, nav);
217228

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

227-
if (_context.Annotations.IsMarked (property))
228-
LogWarning (nav, DiagnosticId.XmlDuplicatePreserveMember, property.FullName);
238+
if (!_preservedMembers.Add (property))
239+
LogDuplicatePreserve(property.FullName, nav);
229240

230241
if (Array.IndexOf (accessors, "all") >= 0) {
231242
ProcessMethodIfNotNull (type, property.GetMethod, nav, customData);

src/tools/illink/src/linker/Linker/MessageContainer.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,11 @@ public static MessageContainer CreateInfoMessage (string text)
253253
return new MessageContainer (MessageCategory.Info, text, null);
254254
}
255255

256+
internal static MessageContainer CreateInfoMessage (MessageOrigin origin, string text)
257+
{
258+
return new MessageContainer (MessageCategory.Info, text, null, "", origin);
259+
}
260+
256261
/// <summary>
257262
/// Create a diagnostics message.
258263
/// </summary>

src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/LinkXmlErrorCases.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ namespace Mono.Linker.Tests.Cases.LinkXml
66
{
77
[SetupLinkerDescriptorFile ("LinkXmlErrorCases.xml")]
88
[SetupLinkerArgument ("--skip-unresolved", "true")]
9+
[SetupLinkerArgument ("--verbose")]
910

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

0 commit comments

Comments
 (0)