Warn on reflection access to instance methods on RUC types #3147
Conversation
This file contains 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
sbomer
requested review from
vitek-karas,
MichalStrehovsky,
tlakollo and
jtschuster
vitek-karas
approved these changes
Dec 2, 2022
jtschuster
reviewed
Dec 2, 2022
jtschuster
reviewed
Dec 2, 2022
jtschuster
reviewed
Dec 2, 2022
tlakollo
reviewed
Dec 2, 2022
- Add link to bug - Remove unused code - Remove left-over incomplete comment - Update comments in modified testcases
tlakollo
pushed a commit
to tlakollo/linker
that referenced
this pull request
Dec 20, 2022
Fixes an analysis hole where reflecting on instance methods of RUC types produced no warnings. This isn't trim-compatible in case the methods are decorated with attributes with RUC constructors, or in case they are invoked on a null instance. We don't warn for RUC attributes on methods in RUC types, but instead prevent reflection access from creating these attributes by warning on the reflection access to methods (including instance methods). This leaves the attribute warnings for fields as-is - so instance fields will get attribute warnings unlike instance methods, because we don't warn on reflection access to instance fields in RUC types. This is currently inconsistent with the analyzer behavior, which doesn't warn for attributes in this case. Commit migrated from dotnet@f3469ca
tlakollo
pushed a commit
to tlakollo/runtime
that referenced
this pull request
Dec 22, 2022
…nker#3147) Fixes an analysis hole where reflecting on instance methods of RUC types produced no warnings. This isn't trim-compatible in case the methods are decorated with attributes with RUC constructors, or in case they are invoked on a null instance. We don't warn for RUC attributes on methods in RUC types, but instead prevent reflection access from creating these attributes by warning on the reflection access to methods (including instance methods). This leaves the attribute warnings for fields as-is - so instance fields will get attribute warnings unlike instance methods, because we don't warn on reflection access to instance fields in RUC types. This is currently inconsistent with the analyzer behavior, which doesn't warn for attributes in this case. Commit migrated from dotnet/linker@f3469ca
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 the hole for methods in #3140. We don't warn for RUC attributes on methods in RUC types, but instead prevent reflection access from creating these attributes by warning on the reflection access to methods (including instance methods).
This leaves the attribute warnings for fields as-is - so instance fields will get attribute warnings unlike instance methods, because we don't warn on reflection access to instance fields in RUC types. This is inconsistent with the analyzer behavior, which doesn't warn for attributes in this case. I may try to address that in a follow-up.