Skip to content
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

Avoid infinite cycle caused by an early binding of DefaultParameterValue attribute. #70242

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented Oct 4, 2023

Fixes #70206.
Also:

  • Add support for using nameof in early bound well-known attributes in VB
  • Address a number of issues with support for Asc, AscW, Chr, ChrW VB functions in early bound well-known attributes

…e attribute.

Fixes dotnet#70206.
Also:
- Add support for using `nameof` in early bound well-known attributes
- Address a number of issues with support for Asc, AscW, Chr, ChrW VB functions in early bound well-known attributes
@AlekseyTs AlekseyTs requested a review from a team as a code owner October 4, 2023 23:41
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Oct 4, 2023
@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler Please review

1 similar comment
@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler Please review

@RikkiGibson RikkiGibson self-assigned this Oct 6, 2023
switch (node.Kind())
BoundExpression result = bindExpressionInternal(node, diagnostics, invoked, indexed);

if (IsEarlyAttributeBinder && result.Kind == BoundKind.MethodGroup && (!IsInsideNameof || EnclosingNameofArgument != node))
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering in what scenario is EnclosingNameofArgument != node needed for correct behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering in what scenario is EnclosingNameofArgument != node needed for correct behavior?

I have taken a conservatives approach to check for scenarios that I think can result in a successful binding in a most precise way. It is quite possible that I am missing some scenario and this condition is too strict, but I cannot think of any at the moment.

@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler For the second review

2 similar comments
@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler For the second review

@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler For the second review

@RikkiGibson RikkiGibson requested a review from a team October 10, 2023 17:53
@AlekseyTs AlekseyTs enabled auto-merge (squash) October 11, 2023 21:40
@AlekseyTs AlekseyTs disabled auto-merge October 12, 2023 15:33
@AlekseyTs AlekseyTs merged commit 6b6c6dc into dotnet:main Oct 12, 2023
25 checks passed
@ghost ghost added this to the Next milestone Oct 12, 2023
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing the method back into DefaultParameterValue crashes
4 participants