Skip to content

Conversation

@AlekseyTs
Copy link
Contributor

This avoids infinite cycle in binding attributes for two or more different symbols.
Fixes #64392.

This avoids infinite cycle in binding attributes for two or more different symbols.
Fixes dotnet#64392.
@AlekseyTs AlekseyTs requested a review from a team as a code owner October 11, 2022 22:29
@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler Please review.

@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler Please review

@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler For the second review.

{
if (attributeMatchesOpt is null)
{
this.PostDecodeWellKnownAttributes(boundAttributes, attributesToBind, diagnostics, symbolPart, wellKnownAttributeData);
Copy link
Member

@333fred 333fred Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it ok to move this from being unconditionally executed to only being executed if there's no matching filter? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it ok to move this from being unconditionally executed to only being executed if there's no matching filter?

The only allowed side effect of this call is error reporting into diagnostics (from doc comment: "This method should not have any side effects on the symbol, i.e. it SHOULD NOT change the symbol state."), and this code path is the only code path that preserves the diagnostics.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation.

Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 1)

@AlekseyTs AlekseyTs merged commit 7afa1ba into dotnet:main Oct 12, 2022
@ghost ghost added this to the Next milestone Oct 12, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.5 P1 Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StackOverflowException in code analyzer for simple class

4 participants