r291465 - PCH: fix a regression that reports a module is defined in both pch and pcm.

Manman Ren via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 9 11:20:19 PST 2017


Author: mren
Date: Mon Jan  9 13:20:18 2017
New Revision: 291465

URL: http://llvm.org/viewvc/llvm-project?rev=291465&view=rev
Log:
PCH: fix a regression that reports a module is defined in both pch and pcm.

In r276159, we started to say that a module X is defined in a pch if we specify
-fmodule-name when building the pch. This caused a regression that reports
module X is defined in both pch and pcm if we generate the pch with
-fmodule-name=X and then in a separate clang invocation, we include the pch and
also import X.pcm.

This patch adds an option CompilingPCH similar to CompilingModule. When we use
-fmodule-name=X while building a pch, modular headers in X will be textually
included and the compiler knows that we are not building module X, so we don't
put module X in SUBMODULE_DEFINITION of the pch.

Differential Revision: http://reviews.llvm.org/D28415

Added:
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/A.h
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.h
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.m
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/D.h
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/module.modulemap
    cfe/trunk/test/Modules/Inputs/pch-with-module-name/test.h
    cfe/trunk/test/Modules/pch-with-module-name.m
Modified:
    cfe/trunk/include/clang/Basic/LangOptions.def
    cfe/trunk/include/clang/Frontend/FrontendActions.h
    cfe/trunk/lib/Frontend/FrontendActions.cpp
    cfe/trunk/lib/Lex/PPDirectives.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=291465&r1=291464&r2=291465&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Mon Jan  9 13:20:18 2017
@@ -146,6 +146,7 @@ LANGOPT(Modules           , 1, 0, "modul
 COMPATIBLE_LANGOPT(ModulesTS  , 1, 0, "C++ Modules TS")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 2, CMK_None,
                     "compiling a module interface")
+BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 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")

Modified: cfe/trunk/include/clang/Frontend/FrontendActions.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendActions.h?rev=291465&r1=291464&r2=291465&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/FrontendActions.h (original)
+++ cfe/trunk/include/clang/Frontend/FrontendActions.h Mon Jan  9 13:20:18 2017
@@ -88,6 +88,8 @@ public:
   static std::unique_ptr<raw_pwrite_stream>
   ComputeASTConsumerArguments(CompilerInstance &CI, StringRef InFile,
                               std::string &Sysroot, std::string &OutputFile);
+
+  bool BeginSourceFileAction(CompilerInstance &CI, StringRef Filename) override;
 };
 
 class GenerateModuleAction : public ASTFrontendAction {

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=291465&r1=291464&r2=291465&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Mon Jan  9 13:20:18 2017
@@ -127,6 +127,12 @@ GeneratePCHAction::ComputeASTConsumerArg
   return OS;
 }
 
+bool GeneratePCHAction::BeginSourceFileAction(CompilerInstance &CI,
+                                              StringRef Filename) {
+  CI.getLangOpts().CompilingPCH = true;
+  return true;
+}
+
 std::unique_ptr<ASTConsumer>
 GenerateModuleAction::CreateASTConsumer(CompilerInstance &CI,
                                         StringRef InFile) {

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=291465&r1=291464&r2=291465&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Jan  9 13:20:18 2017
@@ -1996,10 +1996,12 @@ void Preprocessor::HandleIncludeDirectiv
 
   // Ask HeaderInfo if we should enter this #include file.  If not, #including
   // this file will have no effect.
+  bool SkipHeader = false;
   if (ShouldEnter &&
       !HeaderInfo.ShouldEnterIncludeFile(*this, File, isImport,
                                          SuggestedModule.getModule())) {
     ShouldEnter = false;
+    SkipHeader = true;
     if (Callbacks)
       Callbacks->FileSkipped(*File, FilenameTok, FileCharacter);
   }
@@ -2008,6 +2010,14 @@ void Preprocessor::HandleIncludeDirectiv
   if (!ShouldEnter) {
     // If this is a module import, make it visible if needed.
     if (auto *M = SuggestedModule.getModule()) {
+      // When building a pch, -fmodule-name tells the compiler to textually
+      // include headers in the specified module. But it is possible that
+      // ShouldEnter is false because we are skipping the header. In that
+      // case, We are not importing the specified module.
+      if (SkipHeader && getLangOpts().CompilingPCH &&
+          M->getTopLevelModuleName() == getLangOpts().CurrentModule)
+        return;
+
       makeModuleVisible(M, HashLoc);
 
       if (IncludeTok.getIdentifierInfo()->getPPKeywordID() !=
@@ -2032,6 +2042,13 @@ void Preprocessor::HandleIncludeDirectiv
 
   // Determine if we're switching to building a new submodule, and which one.
   if (auto *M = SuggestedModule.getModule()) {
+    // When building a pch, -fmodule-name tells the compiler to textually
+    // include headers in the specified module. We are not building the
+    // specified module.
+    if (getLangOpts().CompilingPCH &&
+        M->getTopLevelModuleName() == getLangOpts().CurrentModule)
+      return;
+
     assert(!CurSubmodule && "should not have marked this as a module yet");
     CurSubmodule = M;
 

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=291465&r1=291464&r2=291465&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Mon Jan  9 13:20:18 2017
@@ -4654,17 +4654,6 @@ uint64_t ASTWriter::WriteASTCore(Sema &S
   // If we're emitting a module, write out the submodule information.  
   if (WritingModule)
     WriteSubmodules(WritingModule);
-  else if (!getLangOpts().CurrentModule.empty()) {
-    // If we're building a PCH in the implementation of a module, we may need
-    // the description of the current module.
-    //
-    // FIXME: We may need other modules that we did not load from an AST file,
-    // such as if a module declares a 'conflicts' on a different module.
-    Module *M = PP.getHeaderSearchInfo().getModuleMap().findModule(
-        getLangOpts().CurrentModule);
-    if (M && !M->IsFromModuleFile)
-      WriteSubmodules(M);
-  }
 
   Stream.EmitRecord(SPECIAL_TYPES, SpecialTypes);
 

Added: cfe/trunk/test/Modules/Inputs/pch-with-module-name/A.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/pch-with-module-name/A.h?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/pch-with-module-name/A.h (added)
+++ cfe/trunk/test/Modules/Inputs/pch-with-module-name/A.h Mon Jan  9 13:20:18 2017
@@ -0,0 +1 @@
+// in pch

Added: cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.h?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.h (added)
+++ cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.h Mon Jan  9 13:20:18 2017
@@ -0,0 +1 @@
+#include "D.h"

Added: cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.m?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.m (added)
+++ cfe/trunk/test/Modules/Inputs/pch-with-module-name/C.m Mon Jan  9 13:20:18 2017
@@ -0,0 +1 @@
+//empty

Added: cfe/trunk/test/Modules/Inputs/pch-with-module-name/D.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/pch-with-module-name/D.h?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/pch-with-module-name/D.h (added)
+++ cfe/trunk/test/Modules/Inputs/pch-with-module-name/D.h Mon Jan  9 13:20:18 2017
@@ -0,0 +1 @@
+//empty

Added: cfe/trunk/test/Modules/Inputs/pch-with-module-name/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/pch-with-module-name/module.modulemap?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/pch-with-module-name/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/pch-with-module-name/module.modulemap Mon Jan  9 13:20:18 2017
@@ -0,0 +1,9 @@
+module CloudKit {
+  header "C.h"
+  export *
+}
+
+module Contacts {
+  header "D.h"
+  export *
+}

Added: cfe/trunk/test/Modules/Inputs/pch-with-module-name/test.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/pch-with-module-name/test.h?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/pch-with-module-name/test.h (added)
+++ cfe/trunk/test/Modules/Inputs/pch-with-module-name/test.h Mon Jan  9 13:20:18 2017
@@ -0,0 +1 @@
+#include "A.h"

Added: cfe/trunk/test/Modules/pch-with-module-name.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pch-with-module-name.m?rev=291465&view=auto
==============================================================================
--- cfe/trunk/test/Modules/pch-with-module-name.m (added)
+++ cfe/trunk/test/Modules/pch-with-module-name.m Mon Jan  9 13:20:18 2017
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/pch-with-module-name -emit-pch -o %t-A.pch %S/Inputs/pch-with-module-name/test.h -fmodule-name=Contacts -x objective-c-header
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/pch-with-module-name -include-pch %t-A.pch %s -fsyntax-only -fmodule-name=Contacts -verify
+// expected-no-diagnostics 
+#include "C.h"




More information about the cfe-commits mailing list