Fix warning origin for DAM on types #2162
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm not seeing an easy way to fix the suppression issue so I'd like to leave that for a separate change (filed #2163) |
vitek-karas
reviewed
Jul 26, 2021
src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
Outdated
Show resolved
Hide resolved
|
Actually the interaction with RUC go so complex that I just added a design doc (it will be easier to discuss that in its own PR): #2168 |
sbomer
force-pushed
the
typeHierarchyWarnings
branch
from
6a24687 to
9eb043c
Compare
vitek-karas
approved these changes
Jul 27, 2021
test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
Show resolved
Hide resolved
test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
Show resolved
Hide resolved
test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
Outdated
Show resolved
Hide resolved
sbomer
force-pushed
the
typeHierarchyWarnings
branch
from
2e5ba55 to
15cffe2
Compare
Closed
sbomer
added a commit
to sbomer/runtime
that referenced
this pull request
Jul 29, 2021
dotnet/linker#2145 warns when accessing members annotated with DynamicallyAccessedMembers using reflection, and dotnet/linker#2162 updates the warning origin of warnings for DynamicallyAccessedMembers on types. This adds suppressions for the new warnings.
Messages for these warnings can not be suppressed at the member level due to issues described in dotnet#2163. Also fix expected warnings for properties, revert ResultChecker changes, and remove redundant test.
- Include "Attribute" in attribute type names - Add comments about why we want to report warnings - Add comment about the interaction with RUC on types
- Clean up some test attributes - Add test with DAM field on annotated PublicMethods type - Add link to issue about duplicate warnings Also use replace DAMT with DAM in tests.
- Don't warn on non-virtual methods that don't have return annotations - Use the helpers introduced in dotnet#2145
Clarify that getter without annotated return value doesn't warn because it's non-virtual
sbomer
force-pushed
the
typeHierarchyWarnings
branch
from
15cffe2 to
e08cded
Compare
sbomer
added a commit
to dotnet/runtime
that referenced
this pull request
Aug 3, 2021
* Update linker warning suppressions dotnet/linker#2145 warns when accessing members annotated with DynamicallyAccessedMembers using reflection, and dotnet/linker#2162 updates the warning origin of warnings for DynamicallyAccessedMembers on types. This adds suppressions for the new warnings. * Add windows-specific suppressions * Update dependencies from https://github.com/mono/linker build 20210729.2 Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21379.2 * Fix failures and address feedback - Annotate mono's EnumBuilder and TypeBuilder - Add (non-unique) readable short names to the warning codes * Update dependencies from https://github.com/mono/linker build 20210730.2 Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21380.2 * Suppress IL2111 * Update dependencies from https://github.com/mono/linker build 20210802.2 Microsoft.NET.ILLink.Tasks From Version 6.0.100-preview.6.21378.1 -> To Version 6.0.100-preview.6.21402.2 * Feedback - Suppress IL2111 in trimming tests - Remove unnecessary DynamicDependency - Fix indentation * Update readable warning names Co-authored-by: Sven Boemer <sbomer@gmail.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
agocke
pushed a commit
to dotnet/runtime
that referenced
this pull request
Nov 16, 2022
* Fix warning origin for DAM on types * Update tests with link to suppression issue Messages for these warnings can not be suppressed at the member level due to issues described in dotnet/linker#2163. Also fix expected warnings for properties, revert ResultChecker changes, and remove redundant test. * PR feedback - Include "Attribute" in attribute type names - Add comments about why we want to report warnings - Add comment about the interaction with RUC on types * Add testcase for nested type with default ctor * PR feedback - Clean up some test attributes - Add test with DAM field on annotated PublicMethods type - Add link to issue about duplicate warnings Also use replace DAMT with DAM in tests. * Use member-level suppressions in tests * Unify with behavior for DAM warnings - Don't warn on non-virtual methods that don't have return annotations - Use the helpers introduced in dotnet/linker#2145 * PR feedback Clarify that getter without annotated return value doesn't warn because it's non-virtual Commit migrated from dotnet/linker@aec70d0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2136.
The warnings pretty much work as described in #2136 (comment), and the scenario from #2136 (comment) will warn on the derived type. There's still an issue where the member warnings can't be suppressed on the member because we haven't marked custom attributes at that point.