Design proposal for DAM-on-type and RUC interactions #2168
Conversation
| [DAM] | ||
| public Type FieldWithDAM; | ||
|
|
||
| // IL2112 - we must do this here, since we won't be able to see the RUC anywhere else |
There was a problem hiding this comment.
If the type also has explicit RUC annotated methods, with the warn-on-member approach I would expect the warning to originate on the method itself. Then I would expect the same for the implicitly RUC methods implied by the RUC on type.
There was a problem hiding this comment.
I guess this goes back to the thing about us marking all members of nested types - which will produce warnings.
|
|
||
| // No warnings - ??? Probably ??? - I think the DAM on the outer type effectively implies DAM(All) on the nested type | ||
| // and thus declaring it explicitly has no effect. But this needs validation specifically for cases where the outer type | ||
| // has DAM(PublicNestedTypes) on it (the new behavior should still imply All on the nested type in such case I think) |
There was a problem hiding this comment.
Agreed, a DAM nested type shouldn't warn because of the outer DAM.
A related question is whether RUC members of the nested DAM type should produce two warnings - one for the outer DAM, one for the inner DAM. I think it should - while the outer DAM effectively implies the inner DAM, it's possible that the app only uses the nested type's DAM requirements (they are really separate references to the RUC member).
There was a problem hiding this comment.
Agreed - I would not worry about producing "duplicate" warnings at this point - that's something we can worry about later if needed.
|
I think this should also mention somewhere that suppressing these warnings is usually an analysis hole since they are completely disconnected from the GetType callsite: [DAM(All)]
class Library {
[UnconditionalSuppressMessage("", "IL2112")]
[RUC]
static void RUCMethod() {}
}
class Program {
static Foo f;
static void Main() {
f.GetType().GetMethod("RUCMethod").Invoke(null, null);
}
} |
|
I added the warning - that's actually another good reason to not suppress these warnings due to RUC. |
Co-authored-by: Sven Boemer <sbomer@gmail.com>
vitek-karas
force-pushed
the
DAMOnTypeRUCInteraction
branch
from
f8826d6 to
d4f9255
Compare
Design proposal for DAM-on-type and RUC interactions (dotnet#2168) Co-authored-by: Sven Boemer <sbomer@gmail.com> Fix analyzer nullref on assembly attribute (dotnet#2534) The analyzer runs for property assignment operations, including those in attributes. To check whether the property assignment should warn, we look for RUC on the containing symbol. However, assembly-level attributes are contained in the global namespace, which has a null containing type. Create a schema for the LinkAttribute XML files (dotnet#2500) Adds xml schema for ILLink.LinkAttributes.xml, and adds relative paths to the schema in xml used in unit tests to enable linting. [main] Update dependencies from dotnet/runtime (dotnet#2539) [main] Update dependencies from dotnet/runtime Title and message resources should be enforced to exist to prevent printing empty messages (dotnet#2538) Currently, missing resources will default to an empty string printing an empty message for the diagnostics. Code should not try to gracefully handle errors. Included in this PR: - DiagnosticString should be able to locate a title and message resource strings otherwise it throws - Add title resource strings for all missing diagnostic Ids Update dependencies from https://github.com/dotnet/runtime build 20220123.5 (dotnet#2542) [main] Update dependencies from dotnet/runtime Update dependencies from https://github.com/dotnet/arcade build 20220121.6 (dotnet#2541) [main] Update dependencies from dotnet/arcade Analyzer warns when Requires... Attribute is on a static constructor (dotnet#2455) Adds warnings in the analyzer for IL2116 (RUC not allowed on static ctor) and adds IL2117 (RequiresDynamicCode not allowed on static ctor) and IL2118 (RequiresAssemblyFiles not allowed on static ctor). Fixes ExpectedWarning not having effects on constructors by adding a Visit override in the compilation tree walker in the test infra. Adds case in GetDisplayName to differentiate .cctor's and .ctor's. Linker doesn't seem to be checking ctors for calls to methods with RUC. This commit removes the check for the expected warnings in the linker until the bug is fixed.
) Co-authored-by: Sven Boemer <sbomer@gmail.com> Commit migrated from dotnet/linker@8721f1c
No description provided.