manmanren created this revision.
manmanren added a reviewer: benlangmuir.
manmanren added a subscriber: cfe-commits.

With PCH+Module, sometimes compiler gives a hard error:
"Module file ‘<some-file path>.pcm' is out of date and needs to be rebuilt"

This happens when we have a PCH importing a module and the module gets 
overwritten by another compiler instance after we build the pch (one example is 
that both compiler instances hash to the same pcm file but use different 
diagnostic options). When we try to load the pch later on, the compiler notices 
that the imported module is out of date (modification date, size do not match)  
but it can't handle this out of date pcm (i.e it does not know how to rebuild 
the pch).

This patch introduces a new command line option so for PCH + module, we can 
turn on this option and if two compiler instances only differ in diagnostic 
options, the latter instance will not invalidate the original pcm.


https://reviews.llvm.org/D22773

Files:
  include/clang/Driver/Options.td
  include/clang/Lex/HeaderSearchOptions.h
  include/clang/Serialization/ASTReader.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/Driver/modules.m
  test/Modules/Inputs/DiagOutOfDate.h
  test/Modules/Inputs/module.map
  test/Modules/Inputs/pch-import-module-out-of-date.pch
  test/Modules/diagnostic-options-out-of-date.m

Index: test/Modules/diagnostic-options-out-of-date.m
===================================================================
--- test/Modules/diagnostic-options-out-of-date.m
+++ test/Modules/diagnostic-options-out-of-date.m
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -Werror -Wno-conversion -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs %s
+// RUN: %clang_cc1 -Werror -Wno-conversion -emit-pch -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -o %t.pch -I %S/Inputs -x objective-c-header %S/Inputs/pch-import-module-out-of-date.pch
+// RUN: %clang_cc1 -Werror -Wconversion -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs %s -fmodules-disable-diagnostic-validation
+// RUN: %clang_cc1 -Werror -Wno-conversion -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -fsyntax-only -I %S/Inputs -include-pch %t.pch %s -verify -fmodules-disable-diagnostic-validation
+// expected-no-diagnostics
+
+@import DiagOutOfDate;
Index: test/Modules/Inputs/pch-import-module-out-of-date.pch
===================================================================
--- test/Modules/Inputs/pch-import-module-out-of-date.pch
+++ test/Modules/Inputs/pch-import-module-out-of-date.pch
@@ -0,0 +1 @@
+@import DiagOutOfDate;
Index: test/Modules/Inputs/module.map
===================================================================
--- test/Modules/Inputs/module.map
+++ test/Modules/Inputs/module.map
@@ -418,3 +418,7 @@
 module MacroFabs1 {
   header "MacroFabs1.h"
 }
+
+module DiagOutOfDate {
+  header "DiagOutOfDate.h"
+}
Index: test/Modules/Inputs/DiagOutOfDate.h
===================================================================
--- test/Modules/Inputs/DiagOutOfDate.h
+++ test/Modules/Inputs/DiagOutOfDate.h
@@ -0,0 +1 @@
+const int a = 1;
Index: test/Driver/modules.m
===================================================================
--- test/Driver/modules.m
+++ test/Driver/modules.m
@@ -33,6 +33,12 @@
 // RUN: %clang -fmodules-validate-system-headers -### %s 2>&1 | FileCheck -check-prefix=MODULES_VALIDATE_SYSTEM_HEADERS %s
 // MODULES_VALIDATE_SYSTEM_HEADERS: -fmodules-validate-system-headers
 
+// RUN: %clang -### %s 2>&1 | FileCheck -check-prefix=MODULES_DISABLE_DIAGNOSTIC_VALIDATION_DEFAULT %s
+// MODULES_DISABLE_DIAGNOSTIC_VALIDATION_DEFAULT-NOT: -fmodules-disable-diagnostic-validation
+
+// RUN: %clang -fmodules-disable-diagnostic-validation -### %s 2>&1 | FileCheck -check-prefix=MODULES_DISABLE_DIAGNOSTIC_VALIDATION %s
+// MODULES_DISABLE_DIAGNOSTIC_VALIDATION: -fmodules-disable-diagnostic-validation
+
 // RUN: %clang -fmodules -fmodule-map-file=foo.map -fmodule-map-file=bar.map -### %s 2>&1 | FileCheck -check-prefix=CHECK-MODULE-MAP-FILES %s
 // CHECK-MODULE-MAP-FILES: "-fmodules"
 // CHECK-MODULE-MAP-FILES: "-fmodule-map-file=foo.map"
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2084,7 +2084,7 @@
 ASTReader::ASTReadResult ASTReader::ReadOptionsBlock(
     BitstreamCursor &Stream, unsigned ClientLoadCapabilities,
     bool AllowCompatibleConfigurationMismatch, ASTReaderListener &Listener,
-    std::string &SuggestedPredefines) {
+    std::string &SuggestedPredefines, bool ValidateDiagnosticOptions) {
   if (Stream.EnterSubBlock(OPTIONS_BLOCK_ID))
     return Failure;
 
@@ -2128,7 +2128,8 @@
 
     case DIAGNOSTIC_OPTIONS: {
       bool Complain = (ClientLoadCapabilities & ARR_OutOfDate) == 0;
-      if (!AllowCompatibleConfigurationMismatch &&
+      if (ValidateDiagnosticOptions &&
+          !AllowCompatibleConfigurationMismatch &&
           ParseDiagnosticOptions(Record, Complain, Listener))
         return OutOfDate;
       break;
@@ -2255,10 +2256,13 @@
           // FIXME: Allow this for files explicitly specified with -include-pch.
           bool AllowCompatibleConfigurationMismatch =
               F.Kind == MK_ExplicitModule;
+          const HeaderSearchOptions &HSOpts =
+              PP.getHeaderSearchInfo().getHeaderSearchOpts();
 
           Result = ReadOptionsBlock(Stream, ClientLoadCapabilities,
                                     AllowCompatibleConfigurationMismatch,
-                                    *Listener, SuggestedPredefines);
+                                    *Listener, SuggestedPredefines,
+                                    HSOpts.ModulesValidateDiagnosticOptions);
           if (Result == Failure) {
             Error("malformed block record in AST file");
             return Result;
@@ -4195,7 +4199,7 @@
     StringRef Filename, FileManager &FileMgr,
     const PCHContainerReader &PCHContainerRdr,
     bool FindModuleFileExtensions,
-    ASTReaderListener &Listener) {
+    ASTReaderListener &Listener, bool ValidateDiagnosticOptions) {
   // Open the AST file.
   // FIXME: This allows use of the VFS; we do not allow use of the
   // VFS when actually loading a module.
@@ -4235,7 +4239,8 @@
         std::string IgnoredSuggestedPredefines;
         if (ReadOptionsBlock(Stream, ARR_ConfigurationMismatch | ARR_OutOfDate,
                              /*AllowCompatibleConfigurationMismatch*/ false,
-                             Listener, IgnoredSuggestedPredefines) != Success)
+                             Listener, IgnoredSuggestedPredefines,
+                             ValidateDiagnosticOptions) != Success)
           return true;
         break;
       }
@@ -4408,7 +4413,8 @@
                                ExistingModuleCachePath, FileMgr);
   return !readASTFileControlBlock(Filename, FileMgr, PCHContainerRdr,
                                   /*FindModuleFileExtensions=*/false,
-                                  validator);
+                                  validator,
+                                  /*ValidateDiagnosticOptions=*/true);
 }
 
 ASTReader::ASTReadResult
Index: lib/Frontend/FrontendActions.cpp
===================================================================
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -608,11 +608,15 @@
   llvm::raw_ostream &Out = OutFile.get()? *OutFile.get() : llvm::outs();
 
   Out << "Information for module file '" << getCurrentFile() << "':\n";
+  Preprocessor &PP = getCompilerInstance().getPreprocessor();
   DumpModuleInfoListener Listener(Out);
+  HeaderSearchOptions &HSOpts =
+      PP.getHeaderSearchInfo().getHeaderSearchOpts();
   ASTReader::readASTFileControlBlock(
       getCurrentFile(), getCompilerInstance().getFileManager(),
       getCompilerInstance().getPCHContainerReader(),
-      /*FindModuleFileExtensions=*/true, Listener);
+      /*FindModuleFileExtensions=*/true, Listener,
+      HSOpts.ModulesValidateDiagnosticOptions);
 }
 
 //===----------------------------------------------------------------------===//
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1362,6 +1362,8 @@
   Opts.ModuleCachePath = Args.getLastArgValue(OPT_fmodules_cache_path);
   Opts.ModuleUserBuildPath = Args.getLastArgValue(OPT_fmodules_user_build_path);
   Opts.DisableModuleHash = Args.hasArg(OPT_fdisable_module_hash);
+  Opts.ModulesValidateDiagnosticOptions =
+      !Args.hasArg(OPT_fmodules_disable_diagnostic_validation);
   Opts.ImplicitModuleMaps = Args.hasArg(OPT_fimplicit_module_maps);
   Opts.ModuleMapFileHomeIsCwd = Args.hasArg(OPT_fmodule_map_file_home_is_cwd);
   Opts.ModuleCachePruneInterval =
Index: include/clang/Serialization/ASTReader.h
===================================================================
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -1153,7 +1153,7 @@
   static ASTReadResult ReadOptionsBlock(
       llvm::BitstreamCursor &Stream, unsigned ClientLoadCapabilities,
       bool AllowCompatibleConfigurationMismatch, ASTReaderListener &Listener,
-      std::string &SuggestedPredefines);
+      std::string &SuggestedPredefines, bool ValidateDiagnosticOptions);
   ASTReadResult ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities);
   ASTReadResult ReadExtensionBlock(ModuleFile &F);
   bool ParseLineTable(ModuleFile &F, const RecordData &Record);
@@ -1516,7 +1516,8 @@
   readASTFileControlBlock(StringRef Filename, FileManager &FileMgr,
                           const PCHContainerReader &PCHContainerRdr,
                           bool FindModuleFileExtensions,
-                          ASTReaderListener &Listener);
+                          ASTReaderListener &Listener,
+                          bool ValidateDiagnosticOptions);
 
   /// \brief Determine whether the given AST file is acceptable to load into a
   /// translation unit with the given language and target options.
Index: include/clang/Lex/HeaderSearchOptions.h
===================================================================
--- include/clang/Lex/HeaderSearchOptions.h
+++ include/clang/Lex/HeaderSearchOptions.h
@@ -172,6 +172,8 @@
   /// Whether the module includes debug information (-gmodules).
   unsigned UseDebugInfo : 1;
 
+  unsigned ModulesValidateDiagnosticOptions : 1;
+
   HeaderSearchOptions(StringRef _Sysroot = "/")
       : Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(0),
         ImplicitModuleMaps(0), ModuleMapFileHomeIsCwd(0),
@@ -181,7 +183,7 @@
         UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
         ModulesValidateOncePerBuildSession(false),
         ModulesValidateSystemHeaders(false),
-        UseDebugInfo(false) {}
+        UseDebugInfo(false), ModulesValidateDiagnosticOptions(true) {}
 
   /// AddPath - Add the \p Path path to the specified \p Group list.
   void AddPath(StringRef Path, frontend::IncludeDirGroup Group,
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -857,6 +857,9 @@
   Group<i_Group>, Flags<[CC1Option]>,
   HelpText<"Don't verify input files for the modules if the module has been "
            "successfully validated or loaded during this build session">;
+def fmodules_disable_diagnostic_validation : Flag<["-"], "fmodules-disable-diagnostic-validation">,
+  Group<i_Group>, Flags<[CC1Option]>,
+  HelpText<"Disable validation of the diagnostic options when loading the module">;
 def fmodules_validate_system_headers : Flag<["-"], "fmodules-validate-system-headers">,
   Group<i_Group>, Flags<[CC1Option]>,
   HelpText<"Validate the system headers that a module depends on when loading the module">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to