- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
Avoid marking property/event attributes multiple times #92094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas Issue DetailsThis tracks whether properties have been marked yet, and avoids marking them multiple times. Fixes a recursion issue where marking a property can cause marking of attributes which may cause marking of the same property. I removed the  Fixes #92064 Fixes the illink part of #84620. That issue mentions a problem with the analyzer behavior, but the testcase I'm fixing in this PR doesn't seem to show duplicate warnings from the analyzer. Can we consider #83581 fixed by this change? 
 | 
| @vitek-karas I realized the same bug exists for events so I fixed that the same way - PTAL. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
NativeAot requires a call to GetCustomAttribute for the attributes to be kept.
| /backport to release/8.0 | 
| Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6202438968 | 
This tracks whether properties have been marked yet, and avoids marking them multiple times. Fixes a recursion issue where marking a property can cause marking of attributes which may cause marking of the same property.
I removed the
AlreadyMarkedassert because #84620 propagates theKindfrom methods to properties - so it can look as if a property is being marked from a descriptor the first time it is encountered.Fixes #92064
Fixes the illink part of #83581. That issue mentions a problem with the analyzer behavior, but the testcase I'm fixing in this PR doesn't seem to show duplicate warnings from the analyzer. Can we consider #83581 fixed by this change?