Check attribute suppressions lazily #2171
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
Previously attribute suppressions were populated while marking the attributes. This meant that any suppression checks before the attributes got marked would not see the suppressions. Doing this lazily fixes the problem, but may result in scanning the attributes multiple times. Fixes dotnet#2163 FIxes dotnet#2095 This also fixes an issue where we were not respecting UnconditionalSuppressMessageAttribute injected via XML for anything other than assembly/module level attributes.
mateoatr
reviewed
Jul 27, 2021
mateoatr
reviewed
Jul 27, 2021
- Make helper private
vitek-karas
approved these changes
Jul 27, 2021
Member
vitek-karas
left a comment
vitek-karas
left a comment
There was a problem hiding this comment.
Other than the test for dotnet/runtime#56252 this looks good!
test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/SuppressWarningsInMembersAndTypes.cs
Show resolved
Hide resolved
sbomer
added a commit
to sbomer/linker
that referenced
this pull request
Jul 29, 2021
The change in dotnet#2171 was incorrect because it didn't account for the possibility that the suppressions cache already contains assembly or module suppressions for the provider. This fixes the check by re-using the cache to also track whether we have scanned for suppression attributes on the provider. This caching is necessary since we warn about duplicate suppressions. Providers without any suppressions may still be scanned multiple times since we don't cache the negative result.
sbomer
added a commit
that referenced
this pull request
Aug 11, 2021
* Fix suppression check with assembly suppressions The change in #2171 was incorrect because it didn't account for the possibility that the suppressions cache already contains assembly or module suppressions for the provider. This fixes the check by re-using the cache to also track whether we have scanned for suppression attributes on the provider. This caching is necessary since we warn about duplicate suppressions. Providers without any suppressions may still be scanned multiple times since we don't cache the negative result. * Populate cache eagerly to avoid extra state Now whenever we add a record to the cache, we ensure that all of the suppressions that might apply to the member have been discovered. This way we don't need to track whether we have scanned the suppressed member, but have to do a bit more work up-front. * Use private accessibility For methods only used in UnconditionalSuppressMessageAttributeState
agocke
pushed a commit
to dotnet/runtime
that referenced
this pull request
Nov 16, 2022
* Check attribute suppressions lazily Previously attribute suppressions were populated while marking the attributes. This meant that any suppression checks before the attributes got marked would not see the suppressions. Doing this lazily fixes the problem, but may result in scanning the attributes multiple times. Fixes dotnet/linker#2163 FIxes dotnet/linker#2095 This also fixes an issue where we were not respecting UnconditionalSuppressMessageAttribute injected via XML for anything other than assembly/module level attributes. * Fix formatting * PR feedback - Make helper private * Add test for compiler generated code in copy assembly Commit migrated from dotnet/linker@5c9d308
agocke
pushed a commit
to dotnet/runtime
that referenced
this pull request
Nov 16, 2022
* Fix suppression check with assembly suppressions The change in dotnet/linker#2171 was incorrect because it didn't account for the possibility that the suppressions cache already contains assembly or module suppressions for the provider. This fixes the check by re-using the cache to also track whether we have scanned for suppression attributes on the provider. This caching is necessary since we warn about duplicate suppressions. Providers without any suppressions may still be scanned multiple times since we don't cache the negative result. * Populate cache eagerly to avoid extra state Now whenever we add a record to the cache, we ensure that all of the suppressions that might apply to the member have been discovered. This way we don't need to track whether we have scanned the suppressed member, but have to do a bit more work up-front. * Use private accessibility For methods only used in UnconditionalSuppressMessageAttributeState Commit migrated from dotnet/linker@5d376b1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Previously attribute suppressions were populated while marking
the attributes. This meant that any suppression checks before the
attributes got marked would not see the suppressions. Doing this
lazily fixes the problem, but may result in scanning the attributes
multiple times.
Fixes #2163
FIxes #2095
Fixes dotnet/runtime#56252
This also fixes an issue where we were not respecting
UnconditionalSuppressMessageAttribute injected via
XML for anything other than assembly/module level attributes.