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

Missing overrides on MultiplexASTDeserializationListener #54521

Closed
lingol opened this issue Mar 24, 2022 · 10 comments
Closed

Missing overrides on MultiplexASTDeserializationListener #54521

lingol opened this issue Mar 24, 2022 · 10 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@lingol
Copy link

lingol commented Mar 24, 2022

Hi, I notice a missing override for ASTDeserializationListener::ModuleImportRead() on wrapper MultiplexASTDeserializationListener.
When someone has registered one or more PluginASTAction plugins, this bug will cause the bridging header's imported module not handled correctly for Swift.
@lingol
Copy link
Author

lingol commented Mar 24, 2022

And I did a little dig in, found out that in 2015 a similar case had taken place 5578e44. Makes one wonder.

@EugeneZelenko EugeneZelenko added clang Clang issues not falling into any other category and removed new issue labels Mar 24, 2022
@lingol lingol changed the title Missing overrides on MultiplexASTDeserializationListener Missing overrides on MultiplexASTDeserializationListener Mar 25, 2022
@lingol
Copy link
Author

lingol commented Mar 28, 2022

@graydon You might check this out.

@lgxbslgx
Copy link
Contributor

@lingol @graydon Have you started the work? If not, I would like to take it over.

@lingol
Copy link
Author

lingol commented Mar 31, 2022

No I have not.

@lgxbslgx
Copy link
Contributor

lgxbslgx commented Apr 9, 2022

Revision URL: https://reviews.llvm.org/D123452

@lingol Sorry for the delay. Could I get your help to review and verify the patch? Thanks.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed clang Clang issues not falling into any other category labels Apr 9, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 9, 2022

@llvm/issue-subscribers-clang-frontend

@lingol
Copy link
Author

lingol commented Apr 11, 2022

Revision URL: https://reviews.llvm.org/D123452

@lingol Sorry for the delay. Could I get your help to review and verify the patch? Thanks.

I'm not familiar with the LLVM dev flow. Is it OK for a total stranger to review and verify a patch?

@lgxbslgx
Copy link
Contributor

When someone has registered one or more PluginASTAction plugins, this bug will cause the bridging header's imported module not handled correctly for Swift.

I'm not familiar with the LLVM dev flow. Is it OK for a total stranger to review and verify a patch?

The LLVM committer may review the patch formally in the near future. But it is good for you to verify whether the patch can solve your issue actually, especially the situation you mentioned about Swift. If you don't want to do that, you can wait for the patch to be integrated and the new llvm version to be released.

@nico
Copy link
Contributor

nico commented Apr 11, 2022

d29f8a5

@nico nico closed this as completed Apr 11, 2022
@lingol
Copy link
Author

lingol commented Apr 12, 2022

Thanks a lot. It's the same as my local modification. So Yes, it does fix my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nico @lingol @EugeneZelenko @lgxbslgx @llvmbot