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

[libc++] Narrow the exports for common_type #111681

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Oct 9, 2024

Based on a comment in #99473, it seems like export * may be overkill.

@ldionne ldionne requested a review from a team as a code owner October 9, 2024 14:01
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

Based on a comment in #99473, it seems like export * may be overkill.


Full diff: https://github.com/llvm/llvm-project/pull/111681.diff

1 Files Affected:

  • (modified) libcxx/include/module.modulemap (+3-3)
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 22a1313498e73e..8c17945bad440b 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -73,9 +73,9 @@ module std_core [system] {
     module common_reference                           { header "__type_traits/common_reference.h" }
     module common_type {
       header "__type_traits/common_type.h"
-      // We need to export everything from this module because common_type inherits from __builtin_common_type,
-      // which needs to be re-exported.
-      export *
+      // We need to export those because common_type expands to either of those based on __builtin_common_type.
+      export std_core.type_traits.type_identity
+      export std_core.utility_core.empty
     }
     module conditional                                { header "__type_traits/conditional.h" }
     module conjunction                                { header "__type_traits/conjunction.h" }

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

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

LGTM assuming the CI is happy. I hope this is actually what's needed to make the modules build happy.

libcxx/include/module.modulemap Outdated Show resolved Hide resolved
Based on a comment in llvm#99473, it seems like `export *` may be overkill.
@ldionne ldionne merged commit 9200ade into llvm:main Oct 9, 2024
8 of 10 checks passed
@ldionne ldionne deleted the review/common-type-export branch October 9, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants