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

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4a418e9..882d42d 100644 (file)
--- 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 530f822..801c12a 100644 (file)
--- 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 58a316b..f9dfb81 100644 (file)
--- 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 7507be1..ae1359c 100644 (file)
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2551,7 +2551,7 @@ public:
   /// 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 f5a9698..48328f6 100644 (file)
--- 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 14551fd..67c2946 100644 (file)
--- 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 (file)
index 0000000..2243de1
--- /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 (file)
index 0000000..0cb8c4b
--- /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@textual.h:* {{module Textual does not depend on a module exporting 'a.h'}}
+#include "textual.h"