-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Avoid a ClangImporter dependency on Sema #61847
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
Conversation
@swift-ci please smoke test |
My recent change started linking ClangImporter with Sema, which accidentally introduced a circular dependency between the two. This patch avoid the usage of type checker logic from ClangImporter's automatic protocol conformance logic. rdar://101763817
1b0c19e
to
7f6fcf6
Compare
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Thanks @tshortli! Let me reproduce and fix this locally. |
Thanks! Note that this will only reproduce on Linux, so if you're not set up to develop on Linux it may just be easier to let CI validate. |
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM.
For posterity: I feel like the layering violation here is on Sema. Sema should only depend on ClangModuleUnit
, not the ClangImporter
itself.
This comment was marked as resolved.
This comment was marked as resolved.
0195f26
to
4bdd72f
Compare
@swift-ci please smoke test |
4bdd72f
to
a7f00d6
Compare
@swift-ci please smoke test |
My recent change started linking ClangImporter with Sema, which accidentally introduced a circular dependency between the two.
This patch avoid the usage of type checker logic from ClangImporter's automatic protocol conformance logic.
rdar://101763817