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.

Sign up for GitHub

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

Jump to bottom

Fix syntax generation of explict checked operator from symbol #77102

Merged
merged 2 commits into from
Feb 10, 2025
Merged

Fix syntax generation of explict checked operator from symbol #77102

merged 2 commits into from
Feb 10, 2025

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Feb 7, 2025

@ericstj ericstj requested review from a team as code owners February 7, 2025 01:38
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 7, 2025
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Feb 7, 2025
@ericstj
Copy link
Member Author

ericstj commented Feb 7, 2025

All right, this is fixed now. My bad for misunderstanding the original bug. I was fixing the syntax layer to accept the bad input that SyntaxGenerator was giving it, but the bug was in SyntaxGenerator.

One thing I noticed -- SyntaxGenerator does not have an OperatorDeclaration overload that permits creating an checked explicit operator from anything other than metadata. Not sure I want to fix that here. @CyrusNajmabadi let me know if you prefer we add an overload and what the public API should look like.

@ericstj ericstj changed the title Fix syntax generation of explict operator from symbol Fix syntax generation of explict checked operator from symbol Feb 7, 2025
@ericstj
Copy link
Member Author

ericstj commented Feb 10, 2025

Ping @CyrusNajmabadi can you have another look?

carlossanlop
carlossanlop reviewed Feb 10, 2025
View reviewed changes
Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

The changes look good to me but I'll defer to Cyrus to approve.

@CyrusNajmabadi
Copy link
Member

LGTM. Thanks!

@CyrusNajmabadi CyrusNajmabadi merged commit daae3d8 into dotnet:main Feb 10, 2025
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Feb 10, 2025
This was referenced Feb 12, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlossanlop carlossanlop carlossanlop left review comments

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

Assignees
No one assigned
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Milestone
Next
3 participants
@ericstj @CyrusNajmabadi @carlossanlop