Aot Analyzer shouldn't warn when passing a const string to RequiresDynamicCode applied to Types #84433
Comments
eerhardt
added
the
area-Tools-ILLink
|
Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas Issue DetailsIf I have a class like: [RequiresDynamicCode(RequiresDynamicCodeMessage)]
public class XmlDsigXsltTransform : Transform
{
internal const string RequiresDynamicCodeMessage = "XmlDsigXsltTransform uses XslCompiledTransform which requires dynamic code.";I am getting a warning by the analyzer: Similarly, if I reference the const string from another [RequiresDynamicCode(XmlDsigXsltTransform.RequiresDynamicCodeMessage)]
public class SignedInfo : ICollection
{produces: But if I reference the const string from a [RequiresDynamicCode(XmlDsigXsltTransform.RequiresDynamicCodeMessage)]
internal void LoadXml(XmlElement value)
{I am declaring a
|
|
A simple solution here might be that |
|
This area is somewhat problematic as But they do exist for the source generator, and I've previously mentioned that it could be useful to place Curious on what you think @sbomer @vitek-karas |
RUC on type applies to statics and .ctors - that's the rule. That's why the warning kicks in. The question here is if there's any situation where the .cctor would be triggered by an access to a const field. |
Right, and the warning behavior rests on the definition of consts, from my perspective. If we see If we decide consts are a separate category, we can apply a different rule to them. |
There's been several discussions on this - the problem is that from IL this is really hard to tell because the values are inlined by the compiler and they look like integers (not even typed as the enum) |
|
Sure -- leaving aside the IL analysis, is it useful to consider only having support in the analyzer? |
|
Hmm - maybe in this case it would be beneficial. |
The IL tools (illink, ilc) don't see const field references in code, since the compiler inlines the values. But analyzer does see them. If the const field is on a type with Requires attribute, that access is reported as a warning, which is only produced by the analyzer. This adds tests for these cases, no product changes. Tests for dotnet#84433
#84452) The IL tools (illink, ilc) don't see const field references in code, since the compiler inlines the values. But analyzer does see them. If the const field is on a type with Requires attribute, that access is reported as a warning, which is only produced by the analyzer. This adds tests for these cases, no product changes. Tests for #84433
|
The only reason why we warn on access to static fields on RUC types is cctor - if there's a cctor, the field access would trigger it. But literal fields don't trigger the cctor because they don't actually get accessed. So the reason doesn't apply: there's nothing "unsafe" about data access. I don't think we should warn. Is there any value in hypothetically being able to mark enum values as RUC? I don't see how that would work E2E: Method(SomeEnum.Unsafe);
enum SomeEnum { Safe, [RUC] Unsafe }
static void Method(SomeEnum val) { ... }So source analyzer would warn here because it sees SomeEnum.Unsafe being used. But IL analysis doesn't see it and won't warn. So we need to do something so that IL analysis can see it. We have no option but to mark If we wanted to go in the direction of carving out safe enum values, we'd probably put the annotations on code. So something like: [RequiresUnreferencedCodeWhen(nameof(val), SomeEnum.Unsafe)]
static void Method(SomeEnum val) { ... } |
Fields can only be annotated with `Requires*` attributes through their type (so RUC on type and so on). But that annotation is actually not guarding access to the field itself, but it's actually guarding access to static members, including static constructor. Constant fields don't trigger static constructor. In fact at runtime constant fields are never accessed by code. The compiler is required to inline their values instead. So IL based scanners will never produce the above warning, only analyzer would. This change modifies the analyzer to stop producing warings for accessing constant fields. Fixes dotnet#84433
ghost
added
the
in-pr
ghost
removed
the
in-pr
…4622) Fields can only be annotated with `Requires*` attributes through their type (so RUC on type and so on). But that annotation is actually not guarding access to the field itself, but it's actually guarding access to static members, including static constructor. Constant fields don't trigger static constructor. In fact at runtime constant fields are never accessed by code. The compiler is required to inline their values instead. So IL based scanners will never produce the above warning, only analyzer would. This change modifies the analyzer to stop producing warings for accessing constant fields. Fixes #84433
ghost
locked as resolved and limited conversation to collaborators
If I have a class like:
I am getting a warning by the analyzer:
Similarly, if I reference the const string from another
RequiresDynamicCodeattribute applied to a class:produces:
But if I reference the const string from a
RequiresDynamicCodeattribute applied to a method, it doesn't warn:I am declaring a
RequiresDynamicCodeattribute, the parameter I pass to the ctor of the attribute should also be considered as being in a "RequiresDynamicCode" scope - since by definition, I'm insideRequiresDynamicCode.@vitek-karas @sbomer
The text was updated successfully, but these errors were encountered: