[clang] c3efd52 - [clang][modules] Disallow importing private framework in the implementation
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 20 13:38:10 PST 2023
Author: Jan Svoboda
Date: 2023-01-20T13:37:36-08:00
New Revision: c3efd52770ca964ba46287298c1d2f7697fd446c
URL: https://github.com/llvm/llvm-project/commit/c3efd52770ca964ba46287298c1d2f7697fd446c
DIFF: https://github.com/llvm/llvm-project/commit/c3efd52770ca964ba46287298c1d2f7697fd446c.diff
LOG: [clang][modules] Disallow importing private framework in the implementation
Whenever we are compiling implementation of a framework (with the `-fmodule-name=FW` option), we never translate `#import <FW/Header.h>` to an import, regardless of whether "Header.h" belongs to "FW" or "FW_Private". For the same reasons, we also disallow `@import FW`. However, we still allow `@import FW_Private`. This patch disallows that a well, to be consistent with the rest of the rules.
Reviewed By: benlangmuir
Differential Revision: https://reviews.llvm.org/D142167
Added:
clang/test/Modules/implementation-of-module-private.m
Modified:
clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Sema/SemaModule.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index fc0ca163fe355..c042cf15d19be 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -469,6 +469,9 @@ class alignas(8) Module {
bool isUnimportable(const LangOptions &LangOpts, const TargetInfo &Target,
Requirement &Req, Module *&ShadowingModule) const;
+ /// Determine whether this module can be built in this compilation.
+ bool isForBuilding(const LangOptions &LangOpts) const;
+
/// Determine whether this module is available for use within the
/// current translation unit.
bool isAvailable() const { return IsAvailable; }
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index aa82a99143379..9c4c83486c2d2 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -148,6 +148,26 @@ bool Module::isUnimportable(const LangOptions &LangOpts,
llvm_unreachable("could not find a reason why module is unimportable");
}
+// The -fmodule-name option tells the compiler to textually include headers in
+// the specified module, meaning Clang won't build the specified module. This
+// is useful in a number of situations, for instance, when building a library
+// that vends a module map, one might want to avoid hitting intermediate build
+// products containing the module map or avoid finding the system installed
+// modulemap for that library.
+bool Module::isForBuilding(const LangOptions &LangOpts) const {
+ StringRef TopLevelName = getTopLevelModuleName();
+ StringRef CurrentModule = LangOpts.CurrentModule;
+
+ // When building framework Foo, we want to make sure that Foo *and*
+ // Foo_Private are textually included and no modules are built for both.
+ if (getTopLevelModule()->IsFramework &&
+ CurrentModule == LangOpts.ModuleName &&
+ !CurrentModule.endswith("_Private") && TopLevelName.endswith("_Private"))
+ TopLevelName = TopLevelName.drop_back(8);
+
+ return TopLevelName == CurrentModule;
+}
+
bool Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target,
Requirement &Req,
UnresolvedHeaderDirective &MissingHeader,
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 41b38b6cfa543..f08d5fe2747f7 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -109,25 +109,6 @@ enum PPElifDiag {
PED_Elifndef
};
-// The -fmodule-name option tells the compiler to textually include headers in
-// the specified module, meaning clang won't build the specified module. This is
-// useful in a number of situations, for instance, when building a library that
-// vends a module map, one might want to avoid hitting intermediate build
-// products containimg the module map or avoid finding the system installed
-// modulemap for that library.
-static bool isForModuleBuilding(Module *M, StringRef CurrentModule,
- StringRef ModuleName) {
- StringRef TopLevelName = M->getTopLevelModuleName();
-
- // When building framework Foo, we wanna make sure that Foo *and* Foo_Private
- // are textually included and no modules are built for both.
- if (M->getTopLevelModule()->IsFramework && CurrentModule == ModuleName &&
- !CurrentModule.endswith("_Private") && TopLevelName.endswith("_Private"))
- TopLevelName = TopLevelName.drop_back(8);
-
- return TopLevelName == CurrentModule;
-}
-
static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
const LangOptions &Lang = PP.getLangOpts();
if (isReservedInAllContexts(II->isReserved(Lang))) {
@@ -2219,14 +2200,13 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
alreadyIncluded(*File))
Action = IncludeLimitReached;
- bool MaybeTranslateInclude = Action == Enter && File && SuggestedModule &&
- !isForModuleBuilding(SuggestedModule.getModule(),
- getLangOpts().CurrentModule,
- getLangOpts().ModuleName);
-
// FIXME: We do not have a good way to disambiguate C++ clang modules from
// C++ standard modules (other than use/non-use of Header Units).
Module *SM = SuggestedModule.getModule();
+
+ bool MaybeTranslateInclude =
+ Action == Enter && File && SM && !SM->isForBuilding(getLangOpts());
+
// Maybe a usable Header Unit
bool UsableHeaderUnit = false;
if (getLangOpts().CPlusPlusModules && SM && SM->isHeaderUnit()) {
@@ -2556,9 +2536,7 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
// that behaves the same as the header would behave in a compilation using
// that PCH, which means we should enter the submodule. We need to teach
// the AST serialization layer to deal with the resulting AST.
- if (getLangOpts().CompilingPCH &&
- isForModuleBuilding(SM, getLangOpts().CurrentModule,
- getLangOpts().ModuleName))
+ if (getLangOpts().CompilingPCH && SM->isForBuilding(getLangOpts()))
return {ImportAction::None};
assert(!CurLexerSubmodule && "should not have marked this as a module yet");
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 8fa31ea9c150b..823a4e9a75743 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -541,7 +541,7 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
// of the same top-level module. Until we do, make it an error rather than
// silently ignoring the import.
// FIXME: Should we warn on a redundant import of the current module?
- if (Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
+ if (Mod->isForBuilding(getLangOpts()) &&
(getLangOpts().isCompilingModule() || !getLangOpts().ModulesTS)) {
Diag(ImportLoc, getLangOpts().isCompilingModule()
? diag::err_module_self_import
diff --git a/clang/test/Modules/implementation-of-module-private.m b/clang/test/Modules/implementation-of-module-private.m
new file mode 100644
index 0000000000000..f6ca2a8809c6b
--- /dev/null
+++ b/clang/test/Modules/implementation-of-module-private.m
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- frameworks/FW.framework/Modules/module.modulemap
+framework module FW {}
+//--- frameworks/FW.framework/Modules/module.private.modulemap
+framework module FW_Private {}
+
+//--- tu.m
+ at import FW_Private; // expected-error{{@import of module 'FW_Private' in implementation of 'FW'; use #import}}
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fimplicit-module-maps \
+// RUN: -fmodule-name=FW -F %t/frameworks %t/tu.m -verify
More information about the cfe-commits
mailing list