-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[ClangImporter] Don't expect unmodularized structs to appear in 2 modules #40091
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
Let's test with the clang fix first. @swift-ci Please smoke test |
@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.
I'm having trouble reasoning about this change. What will the test eventually look like once we're relying on the clang change?
With the clang fix the test should be updated with the following for both importation order.
The type |
I think it'd be better to leave the code unchanged and add a modified version of the error expectation:
Along with a comment explaining that the "*" is transitional (and maybe referencing a relevant bug). That will accept either behavior for now. |
eb72e77
to
c8c3f9b
Compare
That makes sense, thank you @beccadax for the suggestions. I've applied the changes, testing again. swiftlang/llvm-project#3497 |
The swift-corelibs-foundation failure on Linux is expected with the clang fix. @swift-ci Please smoke test |
The clang fix at swiftlang/llvm-project#3497 implements better decl merging, this affects how unmodularized types are imported. This fix triggers an old test explicitly supporting unmodularized code. We believe this test should be updated as the new behavior will prevent possible miscompilations.
We should add an expected error here once the clang fix lands, until them just makes sure the test works both with and without the clang fix.