-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
EnC: Check member modifier updates in semantic analysis #54406
Conversation
@davidwengier Seems like we are missing tests that update attributes on non-type members (specifically events). |
Thanks. Logged #54430 |
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.
The actual modifier update bits looks good. The analyzer changes are difficult to reason about without debugging through, but I trust the tests are doing the right thing, so I'm sure it's fine. Plus you're fairly trustworthy too 😛
AnalyzeCustomAttributes(oldSymbol, newSymbol, capabilities, diagnostics, semanticEdits, syntaxMap, cancellationToken); | ||
if (isDeclarationWithInitializer) | ||
{ | ||
AnalyzeSymbolUpdate(oldSymbol, newSymbol, newDeclaration, capabilities, diagnostics, semanticEdits, syntaxMap, cancellationToken); |
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.
I'm sure this is covered by some other case (and hopefully tests), but this looks like an issue. Whether there is an initializer, we still need to report rude edits for attributes etc., right?
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.
We call DeferConstructorEdit for constructors that emit member initializers and declarations with initializers. This takes care of handling symbol changes (attributes, modifiers) for the latter. The former is handled in AddConstructorEdits where we actually issue edits for the constructor.
Analyzing modifier changes in semantic analysis has multiple advantages:
The PR also fixes a few issues uncovered by the changes in the analyzer.