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

[CIR][Asm] Fix parsing of extra(...) attributes in cir.call #835

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

keryell
Copy link
Contributor

@keryell keryell commented Sep 13, 2024

The parser was looking for extra(...) before the return type while the pretty-printer put it after the return type.
This was breaking the LSP-server for example.
Change the parser behavior accordingly.

Copy link
Collaborator

@seven-mile seven-mile left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it! I also noticed it when I added cc(...) several days ago and planned to solve it recently. ; )

I believe the landing_pad / continue for try_call and cc() also need reordering. Would you like to double check them?

The parser was looking for extra(...) before the return type while the
pretty-printer put it after the return type.
This was breaking the LSP-server for example.
Change the parser behavior accordingly.
@keryell
Copy link
Contributor Author

keryell commented Sep 13, 2024

landing_pad

Surprise! LSP server is choking on clang/test/CIR/IR/call-op-call-conv.cir, as you predicted.
I will look at it later in another PR.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Awesome, LGTM

@bcardosolopes bcardosolopes merged commit 8c017d9 into llvm:main Sep 13, 2024
6 checks passed
@keryell
Copy link
Contributor Author

keryell commented Sep 16, 2024

I believe the landing_pad / continue for try_call and cc() also need reordering. Would you like to double check them?

This should be fixed with #847

Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
The parser was looking for extra(...) before the return type while the
pretty-printer put it after the return type.
This was breaking the LSP-server for example.
Change the parser behavior accordingly.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
The parser was looking for extra(...) before the return type while the
pretty-printer put it after the return type.
This was breaking the LSP-server for example.
Change the parser behavior accordingly.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
The parser was looking for extra(...) before the return type while the
pretty-printer put it after the return type.
This was breaking the LSP-server for example.
Change the parser behavior accordingly.
keryell added a commit to keryell/clangir that referenced this pull request Oct 19, 2024
The parser was looking for extra(...) before the return type while the
pretty-printer put it after the return type.
This was breaking the LSP-server for example.
Change the parser behavior accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants