cjdb created this revision.
cjdb added reviewers: aaron.ballman, rsmith, manojgupta, gbiv, ldionne, EricWF.
Herald added a reviewer: george.burgess.iv.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
libc++ has started splicing standard library headers into much more
fine-grained content for maintainability. It's very likely that outdated
and naive tooling (some of which is outside of LLVM's scope) will
suggest users include things such as `<__algorithm/find.h>` instead of
`<algorithm>`, and Hyrum's law suggests that users will eventually begin
to rely on this without the help of tooling. As such, this commit
intends to protect users from themselves, by making it a hard error for
anyone outside of the standard library to include libc++ detail headers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D105439
Files:
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/lib/Lex/ModuleMap.cpp
clang/test/Modules/Inputs/libcxx-reserved/__libcxx/horde_prime.h
clang/test/Modules/Inputs/libcxx-reserved/__libcxx/princesses/she-ra.h
clang/test/Modules/Inputs/libcxx-reserved/module.modulemap
clang/test/Modules/Inputs/libcxx-reserved/princesses.h
clang/test/Modules/libcxx-reserved.cpp
Index: clang/test/Modules/libcxx-reserved.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/libcxx-reserved.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t \
+// RUN: -fmodule-map-file=%S/Inputs/libcxx-reserved/module.modulemap \
+// RUN: -fmodules-local-submodule-visibility -I%S/Inputs/libcxx-reserved -verify %s
+// RUN: ls %t
+
+#include "princesses.h"
+#include "__libcxx/princesses/she-ra.h"
+// expected-error@-1{{header '__libcxx/princesses/she-ra.h' is a libc++ detail header and can't be included by users; include '<princesses.h>' instead, which is user-facing}}
+#include "__libcxx/horde_prime.h"
+// expected-error@-1{{header '__libcxx/horde_prime.h' is a libc++ detail header and can't be included by users}}
+
+int main()
+{}
Index: clang/test/Modules/Inputs/libcxx-reserved/princesses.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/libcxx-reserved/princesses.h
@@ -0,0 +1,7 @@
+#ifndef PUBLIC_HEADER_1
+#define PUBLIC_HEADER_1
+
+#include "__libcxx/horde_prime.h"
+#include "__libcxx/princesses/she-ra.h"
+
+#endif // PUBLIC_HEADER_1
Index: clang/test/Modules/Inputs/libcxx-reserved/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/libcxx-reserved/module.modulemap
@@ -0,0 +1,12 @@
+module std {
+ module princesses {
+ header "princesses.h"
+ export *
+
+ module __princesses {
+ module she_ra { header "__libcxx/princesses/she-ra.h" }
+ }
+ }
+
+ module __horde_prime { header "__libcxx/horde_prime.h" }
+}
Index: clang/test/Modules/Inputs/libcxx-reserved/__libcxx/princesses/she-ra.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/libcxx-reserved/__libcxx/princesses/she-ra.h
@@ -0,0 +1,6 @@
+#ifndef PRIVATE_HEADER_1
+#define PRIVATE_HEADER_1
+
+using i32 = int;
+
+#endif // PRIVATE_HEADER_1
Index: clang/test/Modules/Inputs/libcxx-reserved/__libcxx/horde_prime.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/libcxx-reserved/__libcxx/horde_prime.h
@@ -0,0 +1 @@
+// This file is intentionally empty
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -470,6 +470,26 @@
return M ? M->getTopLevelModule() : nullptr;
}
+static bool implementationHeaderUsedExternally(const Module *RequestingModule,
+ StringRef Filename) {
+ return Filename.startswith("__libcxx") &&
+ (RequestingModule == nullptr ||
+ RequestingModule->getTopLevelModuleName() != "std");
+}
+
+static std::string FindAppropriateStdlibHeader(const Module *StdlibModule) {
+ const Module *OriginalModule = StdlibModule;
+ while (StdlibModule->Parent && StdlibModule->Parent->Name != "std") {
+ StdlibModule = StdlibModule->Parent;
+ }
+
+ if (StdlibModule == OriginalModule || StdlibModule->Headers[0].empty()) {
+ return "";
+ }
+
+ return StdlibModule->Headers[0][0].NameAsWritten;
+}
+
void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
bool RequestingModuleIsModuleInterface,
SourceLocation FilenameLoc,
@@ -488,6 +508,7 @@
bool Excluded = false;
Module *Private = nullptr;
Module *NotUsed = nullptr;
+ Module *StdlibImplModule = nullptr;
HeadersMap::iterator Known = findKnownHeader(File);
if (Known != Headers.end()) {
@@ -498,6 +519,14 @@
continue;
}
+ // Headers with "__" in their name are reserved for implementation use
+ // only.
+ if (LangOpts.CPlusPlus &&
+ implementationHeaderUsedExternally(RequestingModule, Filename)) {
+ StdlibImplModule = Header.getModule();
+ continue;
+ }
+
// If uses need to be specified explicitly, we are only allowed to return
// modules that are explicitly used by the requesting module.
if (RequestingModule && LangOpts.ModulesDeclUse &&
@@ -520,6 +549,18 @@
return;
}
+ // Header is owned by the standard library
+ if (StdlibImplModule) {
+ const std::string AppropriateHeader =
+ FindAppropriateStdlibHeader(StdlibImplModule);
+ Diags.Report(FilenameLoc,
+ AppropriateHeader.empty()
+ ? diag::err_module_internal_header_use
+ : diag::err_module_internal_header_use_with_user_facing)
+ << Filename << AppropriateHeader;
+ return;
+ }
+
// We have found a module, but we don't use it.
if (NotUsed) {
Diags.Report(FilenameLoc, diag::err_undeclared_use_of_module)
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -793,6 +793,12 @@
"module '%0' conflicts with already-imported module '%1': %2">,
InGroup<ModuleConflict>;
+def err_module_internal_header_use : Error<
+ "header '%0' is a libc++ detail header and can't be included by users">;
+def err_module_internal_header_use_with_user_facing : Error<
+ "header '%0' is a libc++ detail header and can't be included by users; "
+ "include '<%1>' instead, which is user-facing">;
+
// C++20 modules
def err_header_import_semi_in_macro : Error<
"semicolon terminating header import declaration cannot be produced "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
