[clang] a002063 - Enforce module decl-use restrictions and private header restrictions in textual headers
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 6 17:14:04 PDT 2022
Author: Richard Smith
Date: 2022-09-06T17:12:57-07:00
New Revision: a002063de37c00133b93d2ce1f0ea78fc4b807ff
URL: https://github.com/llvm/llvm-project/commit/a002063de37c00133b93d2ce1f0ea78fc4b807ff
DIFF: https://github.com/llvm/llvm-project/commit/a002063de37c00133b93d2ce1f0ea78fc4b807ff.diff
LOG: Enforce module decl-use restrictions and private header restrictions in textual headers
Per the documentation, these restrictions were intended to apply to textual headers but previously this didn't work because we decided there was no requesting module when the `#include` was in a textual header.
A `-cc1` flag is provided to restore the old behavior for transitionary purposes.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D132779
Added:
clang/test/Modules/Inputs/declare-use/textual.h
clang/test/Modules/declare-use-textual.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/PPDirectives.cpp
clang/test/Modules/Inputs/declare-use/module.map
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4a418e976bd4f..882d42d550409 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -138,6 +138,13 @@ Non-comprehensive list of changes in this release
- It's now possible to set the crash diagnostics directory through
the environment variable ``CLANG_CRASH_DIAGNOSTICS_DIR``.
The ``-fcrash-diagnostics-dir`` flag takes precedence.
+- When using header modules, inclusion of a private header and violations of
+ the `use-declaration rules
+ <https://clang.llvm.org/docs/Modules.html#use-declaration>`_ are now
+ diagnosed even when the includer is a textual header. This change can be
+ temporarily reversed with ``-Xclang
+ -fno-modules-validate-textual-header-includes``, but this flag will be
+ removed in a future Clang release.
New Compiler Flags
------------------
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 530f82268eb09..801c12a4b3c77 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -181,6 +181,7 @@ BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "instantiate templates while build
COMPATIBLE_LANGOPT(ModulesDeclUse , 1, 0, "require declaration of module uses")
BENIGN_LANGOPT(ModulesSearchAll , 1, 1, "searching even non-imported modules to find unresolved references")
COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of module uses and all headers to be in modules")
+COMPATIBLE_LANGOPT(ModulesValidateTextualHeaderIncludes, 1, 1, "validation of textual header includes")
BENIGN_LANGOPT(ModulesErrorRecovery, 1, 1, "automatically importing modules as needed when performing error recovery")
BENIGN_LANGOPT(ImplicitModules, 1, 1, "building modules that are not specified via -fmodule-file")
COMPATIBLE_LANGOPT(ModulesLocalVisibility, 1, 0, "local submodule visibility")
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 58a316b0180e6..f9dfb8104395e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2273,6 +2273,12 @@ defm modules_validate_system_headers : BoolOption<"f", "modules-validate-system-
HeaderSearchOpts<"ModulesValidateSystemHeaders">, DefaultFalse,
PosFlag<SetTrue, [CC1Option], "Validate the system headers that a module depends on when loading the module">,
NegFlag<SetFalse, [NoXarchOption]>>, Group<i_Group>;
+def fno_modules_validate_textual_header_includes :
+ Flag<["-"], "fno-modules-validate-textual-header-includes">,
+ Group<f_Group>, Flags<[CC1Option, NoXarchOption]>,
+ MarshallingInfoNegativeFlag<LangOpts<"ModulesValidateTextualHeaderIncludes">>,
+ HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. "
+ "This flag will be removed in a future Clang release.">;
def fvalidate_ast_input_files_content:
Flag <["-"], "fvalidate-ast-input-files-content">,
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 7507be19f0379..ae1359c356a25 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2551,7 +2551,7 @@ class Preprocessor {
/// Find the module that owns the source or header file that
/// \p Loc points to. If the location is in a file that was included
/// into a module, or is outside any module, returns nullptr.
- Module *getModuleForLocation(SourceLocation Loc);
+ Module *getModuleForLocation(SourceLocation Loc, bool AllowTextual);
/// We want to produce a diagnostic at location IncLoc concerning an
/// unreachable effect at location MLoc (eg, where a desired entity was
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index f5a9698c57c07..48328f60a0e7b 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -856,7 +856,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
Tok.getLocation());
}
-Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
+Module *Preprocessor::getModuleForLocation(SourceLocation Loc,
+ bool AllowTextual) {
if (!SourceMgr.isInMainFile(Loc)) {
// Try to determine the module of the include directive.
// FIXME: Look into directly passing the FileEntry from LookupFile instead.
@@ -864,7 +865,7 @@ Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
if (const FileEntry *EntryOfIncl = SourceMgr.getFileEntryForID(IDOfIncl)) {
// The include comes from an included file.
return HeaderInfo.getModuleMap()
- .findModuleForHeader(EntryOfIncl)
+ .findModuleForHeader(EntryOfIncl, AllowTextual)
.getModule();
}
}
@@ -879,7 +880,8 @@ Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
const FileEntry *
Preprocessor::getHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
SourceLocation Loc) {
- Module *IncM = getModuleForLocation(IncLoc);
+ Module *IncM = getModuleForLocation(
+ IncLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
// Walk up through the include stack, looking through textual headers of M
// until we hit a non-textual header that we can #include. (We assume textual
@@ -953,7 +955,8 @@ Optional<FileEntryRef> Preprocessor::LookupFile(
ConstSearchDirIterator CurDirLocal = nullptr;
ConstSearchDirIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
- Module *RequestingModule = getModuleForLocation(FilenameLoc);
+ Module *RequestingModule = getModuleForLocation(
+ FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
// If the header lookup mechanism may be relative to the current inclusion
diff --git a/clang/test/Modules/Inputs/declare-use/module.map b/clang/test/Modules/Inputs/declare-use/module.map
index 14551fde7e13e..67c2946089a04 100644
--- a/clang/test/Modules/Inputs/declare-use/module.map
+++ b/clang/test/Modules/Inputs/declare-use/module.map
@@ -73,3 +73,7 @@ module XN {
module XS {
}
+
+module Textual {
+ textual header "textual.h"
+}
diff --git a/clang/test/Modules/Inputs/declare-use/textual.h b/clang/test/Modules/Inputs/declare-use/textual.h
new file mode 100644
index 0000000000000..2243de1baf9a0
--- /dev/null
+++ b/clang/test/Modules/Inputs/declare-use/textual.h
@@ -0,0 +1 @@
+#include "a.h"
diff --git a/clang/test/Modules/declare-use-textual.cpp b/clang/test/Modules/declare-use-textual.cpp
new file mode 100644
index 0000000000000..0cb8c4b18275c
--- /dev/null
+++ b/clang/test/Modules/declare-use-textual.cpp
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=Textual -I %S/Inputs/declare-use %s -verify
+// RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=Textual -I %S/Inputs/declare-use %s -fno-modules-validate-textual-header-includes
+
+// expected-error at textual.h:* {{module Textual does not depend on a module exporting 'a.h'}}
+#include "textual.h"
More information about the cfe-commits
mailing list