Skip to content

Commit b4488ec

Browse files
vitek-karasvitek-karas
authored
NativeAOT implement warning as errors as a compiler feature (#96567)
Both roslyn and trimmer implement "warn as error" as a compiler feature - specifically the `TreatWarningsAsErrors` property only affects tools which explicitly react to it. There's a related MSBuild command line option which will affect all warnings, but that's not the one VS sets from the UI and what most customers use. See the comments in #94178. This implements the same behavior as the trimmer. It adds 3 new command line options to `ilc` and enable "warn as error" globally and to enable/disable specific warning codes. Note that `illink` implements a more complex command line parsing where order of parameters matters and sometimes the later ones can override the earlier ones. This was trying to emulate `csc` behavior. For `ilc` I don't see a reason to emulate that because running `ilc` on its own is not supported, only going through the SDK, which will never pass the new parameters multiple times. For testing this enables the existing trimmer tests for this functionality. This uncovered a small issue in substitution parser, which is also fixed here. For simplicity the test infra emulates the `illink` command line behavior over `ilc` (which is easy as they're very similar).
1 parent 0f07c39 commit b4488ec

File tree

18 files changed

+180
-18
lines changed

18 files changed

+180
-18
lines changed

‎src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ The .NET Foundation licenses this file to you under the MIT license.
2828
<PublishTrimmed Condition="'$(PublishTrimmed)' == ''">true</PublishTrimmed>
2929
<IlcPgoOptimize Condition="'$(IlcPgoOptimize)' == '' and '$(OptimizationPreference)' == 'Speed'">true</IlcPgoOptimize>
3030
<RunILLink>false</RunILLink>
31+
<IlcTreatWarningsAsErrors Condition="'$(IlcTreatWarningsAsErrors)' == ''">$(TreatWarningsAsErrors)</IlcTreatWarningsAsErrors>
3132
<_IsiOSLikePlatform Condition="'$(_targetOS)' == 'maccatalyst' or $(_targetOS.StartsWith('ios')) or $(_targetOS.StartsWith('tvos'))">true</_IsiOSLikePlatform>
3233
<_IsApplePlatform Condition="'$(_targetOS)' == 'osx' or '$(_IsiOSLikePlatform)' == 'true'">true</_IsApplePlatform>
3334
</PropertyGroup>
@@ -268,6 +269,9 @@ The .NET Foundation licenses this file to you under the MIT license.
268269
<IlcArg Condition="'$(_targetOS)' == 'win' and $(IlcMultiModule) != 'true' and '$(IlcGenerateWin32Resources)' != 'false' and '$(NativeLib)' != 'Static'" Include="--win32resourcemodule:%(ManagedBinary.Filename)" />
269270
<IlcArg Condition="$(IlcDumpIL) == 'true'" Include="--ildump:$(NativeIntermediateOutputPath)%(ManagedBinary.Filename).il" />
270271
<IlcArg Condition="$(NoWarn) != ''" Include='--nowarn:"$([MSBuild]::Escape($(NoWarn)).Replace(`%0A`, ``).Replace(`%0D`, ``))"' />
272+
<IlcArg Condition="$(IlcTreatWarningsAsErrors) == 'true'" Include="--warnaserror" />
273+
<IlcArg Condition="$(WarningsAsErrors) != ''" Include='--warnaserr:"$([MSBuild]::Escape($(WarningsAsErrors)).Replace(`%0A`, ``).Replace(`%0D`, ``))"' />
274+
<IlcArg Condition="$(WarningsNotAsErrors) != ''" Include='--nowarnaserr:"$([MSBuild]::Escape($(WarningsNotAsErrors)).Replace(`%0A`, ``).Replace(`%0D`, ``))"' />
271275
<IlcArg Condition="$(TrimmerSingleWarn) == 'true'" Include="--singlewarn" />
272276
<IlcArg Condition="$(SuppressTrimAnalysisWarnings) == 'true'" Include="--notrimwarn" />
273277
<IlcArg Condition="$(SuppressAotAnalysisWarnings) == 'true'" Include="--noaotwarn" />

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/BodySubstitutionParser.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,15 @@ protected override void ProcessMethod(TypeDesc type, XPathNavigator methodNav, o
7272
_methodSubstitutions.Add(method, BodySubstitution.ThrowingBody);
7373
break;
7474
case "stub":
75-
BodySubstitution stubBody;
75+
BodySubstitution stubBody = null;
7676
if (method.Signature.ReturnType.IsVoid)
7777
stubBody = BodySubstitution.EmptyBody;
7878
else
79-
stubBody = BodySubstitution.Create(TryCreateSubstitution(method.Signature.ReturnType, GetAttribute(methodNav, "value")));
79+
{
80+
object substitution = TryCreateSubstitution(method.Signature.ReturnType, GetAttribute(methodNav, "value"));
81+
if (substitution != null)
82+
stubBody = BodySubstitution.Create(substitution);
83+
}
8084

8185
if (stubBody != null)
8286
{

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logger.cs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ public class Logger
3434
private readonly HashSet<string> _trimWarnedAssemblies = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
3535
private readonly HashSet<string> _aotWarnedAssemblies = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
3636

37+
private readonly bool _treatWarningsAsErrors;
38+
private readonly Dictionary<int, bool> _warningsAsErrors;
39+
3740
public static Logger Null = new Logger(new TextLogWriter(TextWriter.Null), null, false);
3841

3942
public bool IsVerbose { get; }
@@ -46,7 +49,9 @@ public Logger(
4649
bool singleWarn,
4750
IEnumerable<string> singleWarnEnabledModules,
4851
IEnumerable<string> singleWarnDisabledModules,
49-
IEnumerable<string> suppressedCategories)
52+
IEnumerable<string> suppressedCategories,
53+
bool treatWarningsAsErrors,
54+
IDictionary<int, bool> warningsAsErrors)
5055
{
5156
_logWriter = writer;
5257
IsVerbose = isVerbose;
@@ -55,17 +60,19 @@ public Logger(
5560
_singleWarnEnabledAssemblies = new HashSet<string>(singleWarnEnabledModules, StringComparer.OrdinalIgnoreCase);
5661
_singleWarnDisabledAssemblies = new HashSet<string>(singleWarnDisabledModules, StringComparer.OrdinalIgnoreCase);
5762
_suppressedCategories = new HashSet<string>(suppressedCategories, StringComparer.Ordinal);
63+
_treatWarningsAsErrors = treatWarningsAsErrors;
64+
_warningsAsErrors = new Dictionary<int, bool>(warningsAsErrors);
5865
_compilerGeneratedState = ilProvider == null ? null : new CompilerGeneratedState(ilProvider, this);
5966
_unconditionalSuppressMessageAttributeState = new UnconditionalSuppressMessageAttributeState(_compilerGeneratedState, this);
6067
}
6168

62-
public Logger(TextWriter writer, ILProvider ilProvider, bool isVerbose, IEnumerable<int> suppressedWarnings, bool singleWarn, IEnumerable<string> singleWarnEnabledModules, IEnumerable<string> singleWarnDisabledModules, IEnumerable<string> suppressedCategories)
63-
: this(new TextLogWriter(writer), ilProvider, isVerbose, suppressedWarnings, singleWarn, singleWarnEnabledModules, singleWarnDisabledModules, suppressedCategories)
69+
public Logger(TextWriter writer, ILProvider ilProvider, bool isVerbose, IEnumerable<int> suppressedWarnings, bool singleWarn, IEnumerable<string> singleWarnEnabledModules, IEnumerable<string> singleWarnDisabledModules, IEnumerable<string> suppressedCategories, bool treatWarningsAsErrors, IDictionary<int, bool> warningsAsErrors)
70+
: this(new TextLogWriter(writer), ilProvider, isVerbose, suppressedWarnings, singleWarn, singleWarnEnabledModules, singleWarnDisabledModules, suppressedCategories, treatWarningsAsErrors, warningsAsErrors)
6471
{
6572
}
6673

6774
public Logger(ILogWriter writer, ILProvider ilProvider, bool isVerbose)
68-
: this(writer, ilProvider, isVerbose, Array.Empty<int>(), singleWarn: false, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>())
75+
: this(writer, ilProvider, isVerbose, Array.Empty<int>(), singleWarn: false, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, new Dictionary<int, bool>())
6976
{
7077
}
7178

@@ -155,10 +162,13 @@ internal bool IsWarningSuppressed(int code, MessageOrigin origin)
155162
return _unconditionalSuppressMessageAttributeState.IsSuppressed(code, origin);
156163
}
157164

158-
internal static bool IsWarningAsError(int _/*code*/)
165+
internal bool IsWarningAsError(int warningCode)
159166
{
160-
// TODO: warnaserror
161-
return false;
167+
bool value;
168+
if (_treatWarningsAsErrors)
169+
return !_warningsAsErrors.TryGetValue(warningCode, out value) || value;
170+
171+
return _warningsAsErrors.TryGetValue(warningCode, out value) && value;
162172
}
163173

164174
internal bool IsSingleWarn(ModuleDesc owningModule, string messageSubcategory)

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/MessageContainer.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ internal static MessageContainer CreateErrorMessage(MessageOrigin? origin, Diagn
131131
if (TryLogSingleWarning(context, code, origin, subcategory))
132132
return null;
133133

134-
if (Logger.IsWarningAsError(code))
134+
if (context.IsWarningAsError(code))
135135
return new MessageContainer(MessageCategory.WarningAsError, text, code, subcategory, origin);
136136

137137
return new MessageContainer(MessageCategory.Warning, text, code, subcategory, origin);
@@ -148,7 +148,7 @@ internal static MessageContainer CreateErrorMessage(MessageOrigin? origin, Diagn
148148
if (TryLogSingleWarning(context, (int)id, origin, subcategory))
149149
return null;
150150

151-
if (Logger.IsWarningAsError((int)id))
151+
if (context.IsWarningAsError((int)id))
152152
return new MessageContainer(MessageCategory.WarningAsError, id, subcategory, origin, args);
153153

154154
return new MessageContainer(MessageCategory.Warning, id, subcategory, origin, args);

‎src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ILCompilerOptions.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,8 @@ public class ILCompilerOptions
1616
public List<string> Descriptors = new List<string> ();
1717
public bool FrameworkCompilation;
1818
public bool SingleWarn;
19+
public List<string> SubstitutionFiles = new List<string> ();
20+
public bool TreatWarningsAsErrors;
21+
public Dictionary<int, bool> WarningsAsErrors = new Dictionary<int, bool> ();
1922
}
2023
}

‎src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingArgumentBuilder.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ public virtual void AddStripLinkAttributes (bool stripLinkAttributes)
147147

148148
public virtual void AddSubstitutions (string file)
149149
{
150+
Options.SubstitutionFiles.Add (file);
150151
}
151152

152153
public virtual void AddLinkAttributes (string file)
@@ -161,6 +162,30 @@ public virtual void AddAdditionalArgument (string flag, string[] values)
161162
else if (flag == "--singlewarn") {
162163
Options.SingleWarn = true;
163164
}
165+
else if (flag.StartsWith("--warnaserror"))
166+
{
167+
if (flag == "--warnaserror" || flag == "--warnaserror+")
168+
{
169+
if (values.Length == 0)
170+
Options.TreatWarningsAsErrors = true;
171+
else
172+
{
173+
foreach (int warning in ProcessWarningCodes(values))
174+
Options.WarningsAsErrors[warning] = true;
175+
}
176+
177+
}
178+
else if (flag == "--warnaserror-")
179+
{
180+
if (values.Length == 0)
181+
Options.TreatWarningsAsErrors = false;
182+
else
183+
{
184+
foreach (int warning in ProcessWarningCodes(values))
185+
Options.WarningsAsErrors[warning] = false;
186+
}
187+
}
188+
}
164189
}
165190

166191
public virtual void ProcessTestInputAssembly (NPath inputAssemblyPath)
@@ -261,6 +286,23 @@ private static void AppendExpandedPaths (Dictionary<string, string> dictionary,
261286
}
262287
}
263288

289+
private static readonly char[] s_separator = new char[] { ',', ';', ' ' };
290+
291+
private static IEnumerable<int> ProcessWarningCodes(IEnumerable<string> warningCodes)
292+
{
293+
foreach (string value in warningCodes)
294+
{
295+
string[] values = value.Split(s_separator, StringSplitOptions.RemoveEmptyEntries);
296+
foreach (string id in values)
297+
{
298+
if (!id.StartsWith("IL", StringComparison.Ordinal) || !ushort.TryParse(id.AsSpan(2), out ushort code))
299+
continue;
300+
301+
yield return code;
302+
}
303+
}
304+
}
305+
264306
public ILCompilerOptions Build ()
265307
{
266308
var options = Options;

‎src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Linq;
88
using System.Reflection;
99
using System.Runtime.InteropServices;
10+
using System.Xml;
1011
using ILCompiler;
1112
using ILCompiler.Dataflow;
1213
using ILLink.Shared.TrimAnalysis;
@@ -84,15 +85,33 @@ public ILScanResults Trim (ILCompilerOptions options, TrimmingCustomizations? cu
8485
options.SingleWarn,
8586
singleWarnEnabledModules: Enumerable.Empty<string> (),
8687
singleWarnDisabledModules: Enumerable.Empty<string> (),
87-
suppressedCategories: Enumerable.Empty<string> ());
88+
suppressedCategories: Enumerable.Empty<string> (),
89+
treatWarningsAsErrors: options.TreatWarningsAsErrors,
90+
warningsAsErrors: options.WarningsAsErrors);
8891

8992
foreach (var descriptor in options.Descriptors) {
9093
if (!File.Exists (descriptor))
9194
throw new FileNotFoundException ($"'{descriptor}' doesn't exist");
9295
compilationRoots.Add (new ILCompiler.DependencyAnalysis.TrimmingDescriptorNode (descriptor));
9396
}
94-
95-
ilProvider = new FeatureSwitchManager (ilProvider, logger, options.FeatureSwitches, default);
97+
98+
var featureSwitches = options.FeatureSwitches;
99+
BodyAndFieldSubstitutions substitutions = default;
100+
IReadOnlyDictionary<ModuleDesc, IReadOnlySet<string>>? resourceBlocks = default;
101+
foreach (string substitutionFilePath in options.SubstitutionFiles)
102+
{
103+
using FileStream fs = File.OpenRead(substitutionFilePath);
104+
substitutions.AppendFrom(BodySubstitutionsParser.GetSubstitutions(
105+
logger, typeSystemContext, XmlReader.Create(fs), substitutionFilePath, featureSwitches));
106+
107+
fs.Seek(0, SeekOrigin.Begin);
108+
109+
resourceBlocks = ManifestResourceBlockingPolicy.UnionBlockings(resourceBlocks,
110+
ManifestResourceBlockingPolicy.SubstitutionsReader.GetSubstitutions(
111+
logger, typeSystemContext, XmlReader.Create(fs), substitutionFilePath, featureSwitches));
112+
}
113+
114+
ilProvider = new FeatureSwitchManager(ilProvider, logger, featureSwitches, substitutions);
96115

97116
CompilerGeneratedState compilerGeneratedState = new CompilerGeneratedState (ilProvider, logger);
98117

‎src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ internal sealed class ILCompilerRootCommand : CliRootCommand
129129
new("--singlewarnassembly") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "Generate single AOT/trimming warning for given assembly" };
130130
public CliOption<string[]> SingleWarnDisabledAssemblies { get; } =
131131
new("--nosinglewarnassembly") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "Expand AOT/trimming warnings for given assembly" };
132+
public CliOption<bool> TreatWarningsAsErrors { get; } =
133+
new("--warnaserror") { Description = "Treat warnings as errors" };
134+
public CliOption<string[]> WarningsAsErrorsEnable { get; } =
135+
new("--warnaserr") { Description = "Enable treating specific warnings as errors" };
136+
public CliOption<string[]> WarningsAsErrorsDisable { get; } =
137+
new("--nowarnaserr") { Description = "Disable treating specific warnings as errors" };
132138
public CliOption<string[]> DirectPInvokes { get; } =
133139
new("--directpinvoke") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "PInvoke to call directly" };
134140
public CliOption<string[]> DirectPInvokeLists { get; } =
@@ -225,6 +231,9 @@ public ILCompilerRootCommand(string[] args) : base(".NET Native IL Compiler")
225231
Options.Add(NoAotWarn);
226232
Options.Add(SingleWarnEnabledAssemblies);
227233
Options.Add(SingleWarnDisabledAssemblies);
234+
Options.Add(TreatWarningsAsErrors);
235+
Options.Add(WarningsAsErrorsEnable);
236+
Options.Add(WarningsAsErrorsDisable);
228237
Options.Add(DirectPInvokes);
229238
Options.Add(DirectPInvokeLists);
230239
Options.Add(MaxGenericCycleDepth);

‎src/coreclr/tools/aot/ILCompiler/Program.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,18 @@ public int Run()
7575

7676
ILProvider ilProvider = new NativeAotILProvider();
7777

78+
Dictionary<int, bool> warningsAsErrors = new Dictionary<int, bool>();
79+
foreach (int warning in ProcessWarningCodes(Get(_command.WarningsAsErrorsEnable)))
80+
{
81+
warningsAsErrors[warning] = true;
82+
}
83+
foreach (int warning in ProcessWarningCodes(Get(_command.WarningsAsErrorsDisable)))
84+
{
85+
warningsAsErrors[warning] = false;
86+
}
7887
var logger = new Logger(Console.Out, ilProvider, Get(_command.IsVerbose), ProcessWarningCodes(Get(_command.SuppressedWarnings)),
79-
Get(_command.SingleWarn), Get(_command.SingleWarnEnabledAssemblies), Get(_command.SingleWarnDisabledAssemblies), suppressedWarningCategories);
88+
Get(_command.SingleWarn), Get(_command.SingleWarnEnabledAssemblies), Get(_command.SingleWarnDisabledAssemblies), suppressedWarningCategories,
89+
Get(_command.TreatWarningsAsErrors), warningsAsErrors);
8090

8191
// NativeAOT is full AOT and its pre-compiled methods can not be
8292
// thrown away at runtime if they mismatch in required ISAs or

‎src/tests/Directory.Build.targets‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,8 @@
555555
so the default for the test tree is partial. -->
556556
<TrimMode Condition="'$(EnableAggressiveTrimming)' != 'true'">partial</TrimMode>
557557

558+
<IlcTreatWarningsAsErrors>false</IlcTreatWarningsAsErrors>
559+
558560
<IlcToolsPath>$(CoreCLRILCompilerDir)</IlcToolsPath>
559561
<IlcToolsPath Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)' or '$(TargetOS)' != '$(HostOS)' or '$(EnableNativeSanitizers)' != ''">$(CoreCLRCrossILCompilerDir)</IlcToolsPath>
560562
<IlcBuildTasksPath>$(CoreCLRILCompilerDir)netstandard/ILCompiler.Build.Tasks.dll</IlcBuildTasksPath>

0 commit comments

Comments
 (0)