rsmith updated this revision to Diff 457742.
rsmith added a comment.

Document the flag to undo the behavior, and mention that it's going away.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132779/new/

https://reviews.llvm.org/D132779

Files:
  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
  clang/test/Modules/Inputs/declare-use/textual.h
  clang/test/Modules/declare-use-textual.cpp

Index: clang/test/Modules/declare-use-textual.cpp
===================================================================
--- /dev/null
+++ 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"
Index: clang/test/Modules/Inputs/declare-use/textual.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/declare-use/textual.h
@@ -0,0 +1 @@
+#include "a.h"
Index: clang/test/Modules/Inputs/declare-use/module.map
===================================================================
--- clang/test/Modules/Inputs/declare-use/module.map
+++ clang/test/Modules/Inputs/declare-use/module.map
@@ -73,3 +73,7 @@
 
 module XS {
 }
+
+module Textual {
+  textual header "textual.h"
+}
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -856,7 +856,8 @@
         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 @@
     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 @@
 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 @@
   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
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -2546,7 +2546,7 @@
   /// 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
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2283,6 +2283,11 @@
   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.">;
 
 def fvalidate_ast_input_files_content:
   Flag <["-"], "fvalidate-ast-input-files-content">,
Index: clang/include/clang/Basic/LangOptions.def
===================================================================
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -181,6 +181,7 @@
 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")
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -120,6 +120,13 @@
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
+- 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.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D132779: ... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D132... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D132... Aaron Ballman via Phabricator via cfe-commits
    • [PATCH] D132... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D132... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D132... Aaron Ballman via Phabricator via cfe-commits
    • [PATCH] D132... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D132... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to