Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nowarn option #1303

Closed
wants to merge 1 commit into from
Closed
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
12 changes: 11 additions & 1 deletion src/ILLink.Tasks/LinkTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,18 @@ public class ILLink : ToolTask

/// <summary>
/// The directory in which to place linked assemblies.
// Maps to '-out'.
/// Maps to '-out'.
/// </summary>
[Required]
public ITaskItem OutputDirectory { get; set; }

/// <summary>
/// The subset of warnings that have to be turned off.
/// This has no effect if '--verbose' is used. Defaults to 'Analysis'.
/// Maps to '--nowarn'.
/// </summary>
public ITaskItem NoWarn { get; set; }

/// <summary>
/// A list of XML root descriptor files specifying linker
/// roots at a granular level. See the mono/linker
Expand Down Expand Up @@ -303,6 +310,9 @@ protected override string GenerateResponseFileCommands ()
if (OutputDirectory != null)
args.Append ("-out ").AppendLine (Quote (OutputDirectory.ItemSpec));

if (NoWarn != null)
args.Append ("--nowarn ").Append (NoWarn);

// Add global optimization arguments
if (_beforeFieldInit is bool beforeFieldInit)
SetOpt (args, "beforefieldinit", beforeFieldInit);
Expand Down
18 changes: 12 additions & 6 deletions src/linker/Linker.Dataflow/FlowAnnotations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,14 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
paramAnnotations[0] = methodMemberTypes;
}
} else if (methodMemberTypes != DynamicallyAccessedMemberTypes.None) {
_context.LogWarning ($"The DynamicallyAccessedMembersAttribute is only allowed on method parameters, return value or generic parameters.", 2041, method);
_context.LogWarning ($"The DynamicallyAccessedMembersAttribute is only allowed on method parameters, return value or generic parameters.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to worry about localization?

Copy link
Contributor Author

@mateoatr mateoatr Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently don't localize any message outputted by the linker. I sincerely don't know how useful that would be, but might be a good next step after polishing the current messages that we have (#1275).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for .NET 5 - linker is not localized (at all). If we think it's important it would be a relatively non-trivial feature on its own (mostly because there's nothing in that space in linker, so we would have to introduce everything from scratch). So .NET 6 (if we think it's important).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are other parts of the tool chain localized? What happens for Csc.exe?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, csc.exe is fully localized. Visual Studio requires localization in all core languages as a shipping criteria, so it was a hard requirement for us. That doesn't sound like the case for the linker, so let's come back to that in the 6.0 timeframe as Vitek suggests.

2041, method, subcategory: MessageSubCategory.DynamicDependency);
}
} else {
offset = 0;
if (methodMemberTypes != DynamicallyAccessedMemberTypes.None) {
_context.LogWarning ($"The DynamicallyAccessedMembersAttribute is only allowed on method parameters, return value or generic parameters.", 2041, method);
_context.LogWarning ($"The DynamicallyAccessedMembersAttribute is only allowed on method parameters, return value or generic parameters.",
2041, method, subcategory: MessageSubCategory.DynamicDependency);
}
}

Expand Down Expand Up @@ -247,7 +249,8 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
}

if (annotatedMethods.Any (a => a.Method == setMethod)) {
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its setter '{setMethod}', but it already has such attribute on the 'value' parameter.", 2043, setMethod);
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its setter '{setMethod}', but it already has such attribute on the 'value' parameter.",
2043, setMethod, subcategory: MessageSubCategory.DynamicDependency);
} else {
int offset = setMethod.HasImplicitThis () ? 1 : 0;
if (setMethod.Parameters.Count > 0) {
Expand All @@ -274,7 +277,8 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
}

if (annotatedMethods.Any (a => a.Method == getMethod)) {
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its getter '{getMethod}', but it already has such attribute on the return value.", 2043, getMethod);
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its getter '{getMethod}', but it already has such attribute on the return value.",
2043, getMethod, subcategory: MessageSubCategory.DynamicDependency);
} else {
annotatedMethods.Add (new MethodAnnotations (getMethod, null, annotation, null));
}
Expand All @@ -283,15 +287,17 @@ TypeAnnotations BuildTypeAnnotations (TypeDefinition type)
FieldDefinition backingField;
if (backingFieldFromGetter != null && backingFieldFromSetter != null &&
backingFieldFromGetter != backingFieldFromSetter) {
_context.LogWarning ($"Could not find a unique backing field for property '{property.FullName}' to propagate DynamicallyAccessedMembersAttribute. The backing fields from getter '{backingFieldFromGetter.FullName}' and setter '{backingFieldFromSetter.FullName}' are not the same.", 2042, property);
_context.LogWarning ($"Could not find a unique backing field for property '{property.FullName}' to propagate DynamicallyAccessedMembersAttribute. The backing fields from getter '{backingFieldFromGetter.FullName}' and setter '{backingFieldFromSetter.FullName}' are not the same.",
2042, property, subcategory: MessageSubCategory.DynamicDependency);
backingField = null;
} else {
backingField = backingFieldFromGetter ?? backingFieldFromSetter;
}

if (backingField != null) {
if (annotatedFields.Any (a => a.Field == backingField)) {
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its field '{backingField}', but it already has such attribute.", 2043, backingField);
_context.LogWarning ($"Trying to propagate DynamicallyAccessedMemberAttribute from property '{property.FullName}' to its field '{backingField}', but it already has such attribute.",
2043, backingField, subcategory: MessageSubCategory.DynamicDependency);
} else {
annotatedFields.Add (new FieldAnnotation (backingField, annotation));
}
Expand Down
2 changes: 1 addition & 1 deletion src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,7 @@ methodParams[argsParam] is ArrayValue arrayValue &&
message += " " + requiresUnreferencedCode.Url;
}

_context.LogWarning (message, 2026, callingMethodDefinition, operation.Offset);
_context.LogWarning (message, 2026, callingMethodDefinition, operation.Offset, MessageSubCategory.UnreferencedCode);
}

// To get good reporting of errors we need to track the origin of the value for all method calls
Expand Down
6 changes: 4 additions & 2 deletions src/linker/Linker.Steps/DynamicDependencyLookupStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ void ProcessDynamicDependencyAttributes (IMemberDefinition member)
if (!IsPreserveDependencyAttribute (ca.AttributeType))
continue;
#if FEATURE_ILLINK
Context.LogWarning ($"PreserveDependencyAttribute is deprecated. Use DynamicDependencyAttribute instead.", 2033, member);
Context.LogWarning ($"PreserveDependencyAttribute is deprecated. Use DynamicDependencyAttribute instead.",
2033, member, subcategory: MessageSubCategory.PreserveDependency);
#endif
if (ca.ConstructorArguments.Count != 3)
continue;
Expand All @@ -82,7 +83,8 @@ void ProcessDynamicDependencyAttributes (IMemberDefinition member)

var assembly = Context.Resolve (new AssemblyNameReference (dynamicDependency.AssemblyName, new Version ()));
if (assembly == null) {
Context.LogWarning ($"Unresolved assembly '{dynamicDependency.AssemblyName}' in DynamicDependencyAttribute on '{member}'", 2035, member);
Context.LogWarning ($"Unresolved assembly '{dynamicDependency.AssemblyName}' in DynamicDependencyAttribute on '{member}'",
2035, member, subcategory: MessageSubCategory.DynamicDependency);
continue;
}

Expand Down
27 changes: 18 additions & 9 deletions src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,8 @@ void MarkDynamicDependency (DynamicDependency dynamicDependency, IMemberDefiniti
if (dynamicDependency.AssemblyName != null) {
assembly = _context.GetLoadedAssembly (dynamicDependency.AssemblyName);
if (assembly == null) {
_context.LogWarning ($"Unresolved assembly '{dynamicDependency.AssemblyName}' in DynamicDependencyAttribute on '{context}'", 2035, context);
_context.LogWarning ($"Unresolved assembly '{dynamicDependency.AssemblyName}' in DynamicDependencyAttribute on '{context}'",
2035, context, subcategory: MessageSubCategory.DynamicDependency);
return;
}
} else {
Expand All @@ -591,19 +592,22 @@ void MarkDynamicDependency (DynamicDependency dynamicDependency, IMemberDefiniti
if (dynamicDependency.TypeName is string typeName) {
type = DocumentationSignatureParser.GetTypeByDocumentationSignature (assembly, typeName);
if (type == null) {
_context.LogWarning ($"Unresolved type '{typeName}' in DynamicDependencyAttribute on '{context}'", 2036, context);
_context.LogWarning ($"Unresolved type '{typeName}' in DynamicDependencyAttribute on '{context}'",
2036, context, subcategory: MessageSubCategory.DynamicDependency);
return;
}
} else if (dynamicDependency.Type is TypeReference typeReference) {
type = typeReference.Resolve ();
if (type == null) {
_context.LogWarning ($"Unresolved type '{typeReference}' in DynamicDependencyAtribute on '{context}'", 2036, context);
_context.LogWarning ($"Unresolved type '{typeReference}' in DynamicDependencyAtribute on '{context}'",
2036, context, subcategory: MessageSubCategory.DynamicDependency);
return;
}
} else {
type = context.DeclaringType.Resolve ();
if (type == null) {
_context.LogWarning ($"Unresolved type '{context.DeclaringType}' in DynamicDependencyAttribute on '{context}'", 2036, context);
_context.LogWarning ($"Unresolved type '{context.DeclaringType}' in DynamicDependencyAttribute on '{context}'",
2036, context, subcategory: MessageSubCategory.DynamicDependency);
return;
}
}
Expand All @@ -612,14 +616,16 @@ void MarkDynamicDependency (DynamicDependency dynamicDependency, IMemberDefiniti
if (dynamicDependency.MemberSignature is string memberSignature) {
members = DocumentationSignatureParser.GetMembersByDocumentationSignature (type, memberSignature, acceptName: true);
if (!members.Any ()) {
_context.LogWarning ($"No members were resolved for '{memberSignature}'.", 2037, context);
_context.LogWarning ($"No members were resolved for '{memberSignature}'.",
2037, context, subcategory: MessageSubCategory.DynamicDependency);
return;
}
} else {
var memberTypes = dynamicDependency.MemberTypes;
members = DynamicallyAccessedMembersBinder.GetDynamicallyAccessedMembers (type, memberTypes);
if (!members.Any ()) {
_context.LogWarning ($"No members were resolved for '{memberTypes}'.", 2037, context);
_context.LogWarning ($"No members were resolved for '{memberTypes}'.",
2037, context, subcategory: MessageSubCategory.DynamicDependency);
return;
}
}
Expand Down Expand Up @@ -688,7 +694,8 @@ protected virtual void MarkUserDependency (MemberReference context, CustomAttrib
assembly = _context.GetLoadedAssembly (assemblyName);
if (assembly == null) {
_context.LogWarning (
$"Could not resolve '{assemblyName}' assembly dependency specified in a `PreserveDependency` attribute that targets method '{context.FullName}'", 2003, context.Resolve ());
$"Could not resolve '{assemblyName}' assembly dependency specified in a `PreserveDependency` attribute that targets method '{context.FullName}'",
2003, context.Resolve (), subcategory: MessageSubCategory.PreserveDependency);
return;
}
} else {
Expand All @@ -701,7 +708,8 @@ protected virtual void MarkUserDependency (MemberReference context, CustomAttrib

if (td == null) {
_context.LogWarning (
$"Could not resolve '{typeName}' type dependency specified in a `PreserveDependency` attribute that targets method '{context.FullName}'", 2004, context.Resolve ());
$"Could not resolve '{typeName}' type dependency specified in a `PreserveDependency` attribute that targets method '{context.FullName}'",
2004, context.Resolve (), subcategory: MessageSubCategory.PreserveDependency);
return;
}
} else {
Expand Down Expand Up @@ -735,7 +743,8 @@ protected virtual void MarkUserDependency (MemberReference context, CustomAttrib
return;

_context.LogWarning (
$"Could not resolve dependency member '{member}' declared in type '{td.FullName}' specified in a `PreserveDependency` attribute that targets method '{context.FullName}'", 2005, td);
$"Could not resolve dependency member '{member}' declared in type '{td.FullName}' specified in a `PreserveDependency` attribute that targets method '{context.FullName}'",
2005, td, subcategory: MessageSubCategory.PreserveDependency);
}

bool MarkDependencyMethod (TypeDefinition type, string name, string[] signature, in DependencyInfo reason, IMemberDefinition sourceLocationMember)
Expand Down
14 changes: 14 additions & 0 deletions src/linker/Linker/Driver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,17 @@ protected int SetupContext (ILogger customLogger = null)
context.OutputWarningSuppressions = true;
continue;

case "--nowarn":
string noWarnArgument = null;
if (!GetStringParam (token, l => noWarnArgument = l))
return -1;

if (!Enum.TryParse (typeof (NoWarn), noWarnArgument, true, out var noWarnEnum))
return -1;

context.DontWarn = (NoWarn) noWarnEnum;
continue;

case "--version":
Version ();
return 1;
Expand Down Expand Up @@ -998,6 +1009,9 @@ static void Usage ()
Console.WriteLine (" -out PATH Specify the output directory. Defaults to 'output'");
Console.WriteLine (" --about About the {0}", _linker);
Console.WriteLine (" --verbose Log messages indicating progress and warnings");
Console.WriteLine (" --nowarn OPTION Turn off all warnings or a predefined subset. Ignored if 'verbose' is used. Defaults to 'analysis'.");
Console.WriteLine (" all: Disable all warnings");
Console.WriteLine (" analysis: Disable dataflow analysis warnings");
Console.WriteLine (" --version Print the version number of the {0}", _linker);
Console.WriteLine (" -help Lists all linker options");
Console.WriteLine (" @FILE Read response file for more options");
Expand Down
25 changes: 18 additions & 7 deletions src/linker/Linker/LinkContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using Mono.Cecil;
using Mono.Cecil.Cil;

Expand Down Expand Up @@ -174,6 +175,8 @@ public ISymbolWriterProvider SymbolWriterProvider {

public WarningSuppressionWriter WarningSuppressionWriter { get; }

public NoWarn DontWarn { get; set; }

public bool OutputWarningSuppressions { get; set; }

public UnconditionalSuppressMessageAttributeState Suppressions { get; set; }
Expand Down Expand Up @@ -229,6 +232,7 @@ public LinkContext (Pipeline pipeline, AssemblyResolver resolver, ReaderParamete
PInvokes = new List<PInvokeInfo> ();
Suppressions = new UnconditionalSuppressMessageAttributeState (this);
WarningSuppressionWriter = new WarningSuppressionWriter (this);
DontWarn = NoWarn.Analysis;

// See https://github.com/mono/linker/issues/612
const CodeOptimizations defaultOptimizations =
Expand Down Expand Up @@ -484,9 +488,22 @@ public bool IsOptimizationEnabled (CodeOptimizations optimization, AssemblyDefin

public void LogMessage (MessageContainer message)
{
if (!LogMessages || message == MessageContainer.Empty)
if (message == MessageContainer.Empty)
return;

if (!LogMessages && message.Category == MessageCategory.Warning) {
switch (DontWarn) {
case NoWarn.All:
return;

case NoWarn.Analysis:
if (MessageSubCategory.Analysis.Contains (message.SubCategory))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe use if (Array.IndexOf (MessageSubCategory.Analysis, message.SubCategory) > -1) to not have to add Linq to the LinkContext? Also, IndexOf does a faster lookup but not much of a difference. I do get that this is more readable, so I'm also ok if you don't want to change it

return;

break;
}
}

if (OutputWarningSuppressions && message.Category == MessageCategory.Warning && message.Origin?.MemberDefinition != null)
WarningSuppressionWriter.AddWarning (message.Code.Value, message.Origin?.MemberDefinition);

Expand Down Expand Up @@ -519,9 +536,6 @@ public void LogDiagnostic (string message)
/// <param name="subcategory">Optionally, further categorize this warning</param>
public void LogWarning (string text, int code, MessageOrigin origin, string subcategory = MessageSubCategory.None)
{
if (!LogMessages)
return;

var warning = MessageContainer.CreateWarningMessage (this, text, code, origin, subcategory);
LogMessage (warning);
}
Expand Down Expand Up @@ -562,9 +576,6 @@ public void LogWarning (string text, int code, string origin, string subcategory
/// <returns>New MessageContainer of 'Error' category</returns>
public void LogError (string text, int code, string subcategory = MessageSubCategory.None, MessageOrigin? origin = null)
{
if (!LogMessages)
return;

var error = MessageContainer.CreateErrorMessage (text, code, subcategory, origin);
LogMessage (error);
}
Expand Down
2 changes: 1 addition & 1 deletion src/linker/Linker/LoggingReflectionPatternRecorder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void RecognizedReflectionAccessPattern (IMemberDefinition source, Instruc
public void UnrecognizedReflectionAccessPattern (IMemberDefinition source, Instruction sourceInstruction, IMetadataTokenProvider accessedItem, string message)
{
var origin = new MessageOrigin (source, sourceInstruction?.Offset);
_context.LogWarning (message, 2006, origin, "Unrecognized reflection pattern");
_context.LogWarning (message, 2006, origin, MessageSubCategory.UnrecognizedReflectionPattern);
}
}
}
11 changes: 11 additions & 0 deletions src/linker/Linker/MessageSubcategory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,18 @@ namespace Mono.Linker
{
public static class MessageSubCategory
{
public const string DynamicDependency = "Dynamic dependency";
public const string None = "";
public const string PreserveDependency = "Preserve dependency";
public const string UnrecognizedReflectionPattern = "Unrecognized reflection pattern";
public const string UnreferencedCode = "Unreferenced code";
public const string UnresolvedAssembly = "Unresolved assembly";

public static readonly string[] Analysis = {
DynamicDependency,
PreserveDependency,
UnrecognizedReflectionPattern,
UnreferencedCode
};
}
}
16 changes: 16 additions & 0 deletions src/linker/Linker/NoWarn.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Mono.Linker
{
public enum NoWarn
{
// Turn off all warnings.
All,

// Turn off warnings having the DynamicDependency, PreserveDependency,
// UnrecognizedReflectionPattern, or UnreferencedCode subcategory.
Analysis
}
}
2 changes: 2 additions & 0 deletions test/Mono.Linker.Tests.Cases/DataFlow/MethodThisDataFlow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
Expand All @@ -13,6 +14,7 @@
namespace Mono.Linker.Tests.Cases.DataFlow
{
[SkipKeptItemsValidation]
[SetupLinkerArgument ("--verbose")]
public class MethodThisDataFlow
{
public static void Main ()
Expand Down
2 changes: 2 additions & 0 deletions test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
using System.Runtime.CompilerServices;
using System.Text;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;

namespace Mono.Linker.Tests.Cases.DataFlow
{
// Note: this test's goal is to validate that the product correctly reports unrecognized patterns
// - so the main validation is done by the UnrecognizedReflectionAccessPattern attributes.
[SkipKeptItemsValidation]
[SetupLinkerArgument ("--verbose")]
public class PropertyDataFlow
{
public static void Main ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@

namespace Mono.Linker.Tests.Cases.DynamicDependencies
{
[LogContains ("IL2037: Mono.Linker.Tests.Cases.DynamicDependencies.DynamicDependencyMemberSignatureWildcard.Dependency(): No members were resolved for '*'.")]
[SetupCompileBefore ("library.dll", new[] { "Dependencies/DynamicDependencyMethodInAssemblyLibrary.cs" })]
[SetupLinkerArgument ("--verbose")]
[LogContains ("IL2037: Mono.Linker.Tests.Cases.DynamicDependencies.DynamicDependencyMemberSignatureWildcard.Dependency(): No members were resolved for '*'.")]
public class DynamicDependencyMemberSignatureWildcard
{
public static void Main ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

namespace Mono.Linker.Tests.Cases.DynamicDependencies
{
[SetupLinkerArgument ("--verbose")]
[LogContains ("IL2037: Mono.Linker.Tests.Cases.DynamicDependencies.DynamicDependencyMethod.B.Broken(): No members were resolved for 'MissingMethod'.")]
[LogContains ("IL2037: Mono.Linker.Tests.Cases.DynamicDependencies.DynamicDependencyMethod.B.Broken(): No members were resolved for 'Dependency2``1(``0,System.Int32,System.Object)'.")]
[LogContains ("IL2037: Mono.Linker.Tests.Cases.DynamicDependencies.DynamicDependencyMethod.B.Broken(): No members were resolved for '#ctor()'.")]
Expand Down
Loading