Fix warnings for DAMT on interfaces#2210
Merged
sbomer merged 3 commits intodotnet:mainfrom Aug 13, 2021
Merged
Conversation
dotnet#2191 addressed issues with redundant warnings from base types, but interfaces need to be handled specially. There were a few issues with the interface handling in that change: - The binding logic didn't include interfaces for DAM.All when declaredOnly was specified, as @vitek-karas pointed out in dotnet#2206 (comment). This could have led to us not marking interfaces for types derived from a DAMT.All-annotated type. Either way, the binding logic should not select interface _members_ when declaredOnly is specified. - The DAMT-on-type logic had special handling for base types, but didn't separately mark interface members, which is needed when applying annotations DAMT.All or DAMT.Interfaces. This could have led to missing interface impls or interface members for types which have (explicit or inherited) DAMT.All or DAMT.Interfaces annotations. Separately, the bit operations on DAMT also didn't correctly handle cases where there are bits shared by different enum members.
vitek-karas
approved these changes
Aug 13, 2021
src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
Outdated
Show resolved
Hide resolved
src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
Outdated
Show resolved
Hide resolved
Comment on lines
+246
to
+247
| // This may produce redundant warnings when the annotation is DAMT.All or DAMT.PublicConstructors and the base already has a | ||
| // subset of those annotations. |
Member
There was a problem hiding this comment.
Unfortunate, but I would not worry about it for now...
src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
agocke
pushed a commit
to dotnet/runtime
that referenced
this pull request
Nov 16, 2022
* Fix warnings for DAMT on interfaces dotnet/linker#2191 addressed issues with redundant warnings from base types, but interfaces need to be handled specially. There were a few issues with the interface handling in that change: - The binding logic didn't include interfaces for DAM.All when declaredOnly was specified, as @vitek-karas pointed out in dotnet/linker#2206 (comment). This could have led to us not marking interfaces for types derived from a DAMT.All-annotated type. Either way, the binding logic should not select interface _members_ when declaredOnly is specified. - The DAMT-on-type logic had special handling for base types, but didn't separately mark interface members, which is needed when applying annotations DAMT.All or DAMT.Interfaces. This could have led to missing interface impls or interface members for types which have (explicit or inherited) DAMT.All or DAMT.Interfaces annotations. Separately, the bit operations on DAMT also didn't correctly handle cases where there are bits shared by different enum members. * Remove debug output * Apply suggestions from code review Co-authored-by: Vitek Karas <vitek.karas@microsoft.com> Co-authored-by: Vitek Karas <vitek.karas@microsoft.com> Commit migrated from dotnet/linker@c321df2
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
#2191 addressed issues with redundant
warnings from base types, but interfaces need to be handled specially.
There were a few issues with the interface handling in that change:
declaredOnly was specified, as @vitek-karas pointed out in
Use DAM binder for MarkEntireType #2206 (comment).
This could have led to us not marking interfaces for types derived
from a DAMT.All-annotated type. Either way, the binding logic
should not select interface members when declaredOnly is specified.
didn't separately mark interface members, which is needed when
applying annotations DAMT.All or DAMT.Interfaces. This could have led
to missing interface impls or interface members for types which have
(explicit or inherited) DAMT.All or DAMT.Interfaces annotations.
Separately, the bit operations on DAMT also didn't correctly handle cases
where there are bits shared by different enum members.