Pass warning options to ILLink

[sbomer]

The linker is adding options --nowarn, --warnaserror, and --warn to control warning behavior similar to the Roslyn. We need to pass appropriate MSBuild properties as inputs to these in the SDK.

• NoWarn should simply be passed to the linker. It will ignore error codes that don't begin with IL.
• WarningsAsErrors/WarningsNotAsErrors similarly should be passed directly to the linker.
• TreatWarningsAsErrors should set another linker-specific property (tentatively ILLinkTreatWarningsAsErrors) with the same semantics, which gets passed to the linker. This is to allow the setting to be controlled independently for the linker and the compiler if desired.
• AnalysisLevel should set a linker-specific property (tentatively ILLinkWarningVersion), which gets passed to the linker to control warning versions. The linker-specific property is similar to WarningLevel for the compiler (see https://github.com/dotnet/sdk/pull/12422/files). I'm suggesting Version instead of Level because this is more accurate, whereas Roslyn is repurposing an existing option whose meaning used to be different.

The reason I am suggesting ILLink as the prefix rather than Trimmer is that these options are conceptually not specific to trimming - we will use the same properties in the future if we introduce single-file-correctness analysis in the linker.

@samsp-msft again, would appreciate your feedback on the naming.
@vitek-karas @eerhardt

[samsp-msft]

I don't understand the difference between WarningVersion and WarningLevel, and why its version based.

I am assuming the names we are talking about here are in MSBuild, in which case ILLink is more descriptive which is a good thing.

[sbomer]

difference between WarningVersion and WarningLevel, and why its version based.

WarningLevel is the name of the SDK option that controls /warn on csc. This used to just accept warning "levels" 0-4 which would only show warnings with certain severity. With dotnet/roslyn#46148 and in the future it will also accept integers >= 5, which will be used for versioned warnings - so new warnings in future releases will not be emitted if you specify a lower /warn:version. This will be used for back-compat, and the default version will be based on the TFM you are targeting, so that a new SDK targeting an old TFM will pass a /warn:version that produces the same set of warnings produced when that TFM was introduced.

Optionally, the developer will be able to set AnalysisLevel - for example, to a version larger than that determined by the TFM to get more recent warnings for the same project with a newer SDK.

I hope that answers your question - it's described in more detail in #12601, with a modification to the design in dotnet/roslyn#46148. They initially introduced a separate property and command-line, but decided to repurpose the existing /warn/WarningLevel instead.

I am assuming the names we are talking about here are in MSBuild

Yup. I went ahead and called the command-line --warn to match Roslyn, but since the MSBuild property is user-facing I thought we should consider diverging and using WarningVersion instead of WarningLevel. We also have the option of using --warnversion or similar for the command-line. If you have feedback on the command-line part, dotnet/linker#1381 would be the place for it.

[vitek-karas]

I agree with using ILLink prefix for the properties - in my mind the main reason to do this is actually the fact that the warnings when reported are all reported from source ILLink - so it's easy for users to match the warning to the MSBuild property driving the settings for it. (I do agree with the trimmer/linker distinction as well, but to me that's secondary).

Regarding WarningLevel/WarningVersion:

• Will there be any interaction with the WarningLevel MSBuild property used by csc? Or is the only interaction planned to be with the AnalysisLevel property...
• Do we have a feel of what's the proposed VS UI for this (if any)?
• I understand that semantically the meaning is "warning version" and for that WarningVersion would be nice.
• That said based on your description all MSBuild properties involved in this topic are called using the term "level":
  • WarningLevel which is the Roslyn one - for backward compat reasons
  • AnalysisLevel which is the main one - no idea what's the reason for calling this "level"
• So if everything else in the areas is called "level" I would prefer that over the more semantically correct "version". (Pending the VS UI - if the UI uses "version" then that would be a strong argument to also use "version").

[sbomer]

For consistency then let's go with ILLinkWarningLevel. My suggestion is to make ILLinkWarningLevel independent of WarningLevel (no interaction) - both will be based on AnalysisLevel, but will be specific to just one tool (linker/compiler).

From dotnet/roslyn-analyzers#3823 it sounds like the feature is planned to have a VS UI, but it's unclear if they will reuse the "Warning Level" UI now that they are reusing the /warn compiler flag - I am asking about it.

edit: It sounds like the UI will have a drop-down for AnalysisLevel.