Skip to content

[Glibc] Use VFS to inject modulemap into Glibc include path #59846

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

Merged
merged 1 commit into from
Jul 4, 2022

Conversation

egorzhdan
Copy link
Contributor

@egorzhdan egorzhdan commented Jul 1, 2022

This fixes modularization issues caused by the presence of Glibc and libstdc++ in a single context.

Some Glibc headers were getting hijacked by the libstdc++ module, and the decls in them were incorrectly determined to be a part of libstdc++. This caused compiler errors when trying to use those decls.

This also means that we can simplify the actual modulemap file (glibc.modulemap.gyb) and reference the Glibc headers directly from it.

Similar to #58843.

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Jul 1, 2022
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Linux

This will fix modularization issues caused by the presence of Glibc and libstdc++ in a single context.

Some Glibc headers were getting hijacked by the libstdc++ module, and the decls in them were incorrectly determined to be a part of libstdc++. This caused compiler errors when trying to use those decls.

After this change, we will be able to reference Glibc headers directly from the module map, without using an additional header (`SwiftGlibc.h`).
@egorzhdan egorzhdan force-pushed the egorzhdan/glibc-vfs-inject branch from aab2f66 to cf33542 Compare July 1, 2022 22:49
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan marked this pull request as ready for review July 1, 2022 22:51
@egorzhdan egorzhdan requested review from hyp and zoecarver July 4, 2022 09:56
@egorzhdan
Copy link
Contributor Author

cc @buttaface @3405691582

@egorzhdan
Copy link
Contributor Author

I'm going to merge this to unblock other work, but I'm happy to discuss this post-merge.

@egorzhdan egorzhdan merged commit 6170437 into main Jul 4, 2022
@egorzhdan egorzhdan deleted the egorzhdan/glibc-vfs-inject branch July 4, 2022 10:04
@finagolfin
Copy link
Member

I will build this natively on Android with the next trunk snapshot tag, whenever that is, and let you know if it causes any problems.

@finagolfin
Copy link
Member

I think this broke the build on the Android CI, will look into fixing it.

@finagolfin
Copy link
Member

Built this with the latest July 6 trunk source snapshot natively on Android without a problem, but I think it is the cause of the Android CI failures when cross-compiling the stdlib. It most likely has to do with passing the path to the glibc.modulemap back to the clang VFS rather than just adding it as a flag until now. Without building and instrumenting the compiler for linux, I ran some tests and I think it's passing the right paths out of your new getClangInvocationFileMapping() here, but the clangImporter is probably misusing them somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants