clang/Modules: Delay err_module_file_conflict if a diagnostic is in flight

[dexonsmith]

Summary

As part of an audit of whether all errors are being reported from the
ASTReader, delay err_module_file_conflict if a diagnostic is already in
flight when it is hit. This required plumbing an extra argument through
the delayed diagnostic mechanics in DiagnosticsEngine.

[dexonsmith]

Updated header docs for the new delayed diagnostic parameter.

[dexonsmith]

Committed as eef69021607950487a9e4110851a05abb54d0fb6.

clang/include/clang/Basic/Diagnostic.h

private:
   unsigned DelayedDiagID;
 
   /// First string argument for the delayed diagnostic.
   std::string DelayedDiagArg1;
 
   /// Second string argument for the delayed diagnostic.
   std::string DelayedDiagArg2;
 
+  /// Third string argument for the delayed diagnostic.
+  std::string DelayedDiagArg3;
+
   /// Optional flag value.
   ///
   /// Some flags accept values, for instance: -Wframe-larger-than=<value> and
   /// -Rpass=<value>. The content of this string is emitted after the flag name
   /// and '='.
   std::string FlagValue;
 
 public:
public:
   ///
   /// \param Arg1 A string argument that will be provided to the
   /// diagnostic. A copy of this string will be stored in the
   /// DiagnosticsEngine object itself.
   ///
   /// \param Arg2 A string argument that will be provided to the
   /// diagnostic. A copy of this string will be stored in the
   /// DiagnosticsEngine object itself.
+  ///
+  /// \param Arg3 A string argument that will be provided to the
+  /// diagnostic. A copy of this string will be stored in the
+  /// DiagnosticsEngine object itself.
   void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "",
-                            StringRef Arg2 = "");
+                            StringRef Arg2 = "", StringRef Arg3 = "");
 
   /// Clear out the current diagnostic.
   void Clear() { CurDiagID = std::numeric_limits<unsigned>::max(); }
 
   /// Return the value associated with this diagnostic flag.
   StringRef getFlagValue() const { return FlagValue; }
 
 private:

clang/include/clang/Serialization/ASTReader.h

private:
   }
 
   /// Produce an error diagnostic and return true.
   ///
   /// This routine should only be used for fatal errors that have to
   /// do with non-routine failures (e.g., corrupted AST file).
   void Error(StringRef Msg) const;
   void Error(unsigned DiagID, StringRef Arg1 = StringRef(),
-             StringRef Arg2 = StringRef()) const;
+             StringRef Arg2 = StringRef(), StringRef Arg3 = StringRef()) const;
   void Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
              unsigned Select) const;
   void Error(llvm::Error &&Err) const;
 
 public:
   /// Load the AST file and validate its contents against the given
   /// Preprocessor.
   ///

clang/lib/Basic/Diagnostic.cpp

void DiagnosticsEngine::Reset() {
 
   // Create a DiagState and DiagStatePoint representing diagnostic changes
   // through command-line.
   DiagStates.emplace_back();
   DiagStatesByLoc.appendFirst(&DiagStates.back());
 }
 
 void DiagnosticsEngine::SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1,
-                                             StringRef Arg2) {
+                                             StringRef Arg2, StringRef Arg3) {
   if (DelayedDiagID)
     return;
 
   DelayedDiagID = DiagID;
   DelayedDiagArg1 = Arg1.str();
   DelayedDiagArg2 = Arg2.str();
+  DelayedDiagArg3 = Arg3.str();
 }
 
 void DiagnosticsEngine::ReportDelayed() {
   unsigned ID = DelayedDiagID;
   DelayedDiagID = 0;
-  Report(ID) << DelayedDiagArg1 << DelayedDiagArg2;
+  Report(ID) << DelayedDiagArg1 << DelayedDiagArg2 << DelayedDiagArg3;
 }
 
 void DiagnosticsEngine::DiagStateMap::appendFirst(DiagState *State) {
   assert(Files.empty() && "not first");
   FirstDiagState = CurDiagState = State;
   CurDiagStateLoc = SourceLocation();
 }
 

clang/lib/Serialization/ASTReader.cpp

void ASTReader::Error(StringRef Msg) const {
   Error(diag::err_fe_pch_malformed, Msg);
   if (PP.getLangOpts().Modules && !Diags.isDiagnosticInFlight() &&
       !PP.getHeaderSearchInfo().getModuleCachePath().empty()) {
     Diag(diag::note_module_cache_path)
       << PP.getHeaderSearchInfo().getModuleCachePath();
   }
 }
 
-void ASTReader::Error(unsigned DiagID,
-                      StringRef Arg1, StringRef Arg2) const {
+void ASTReader::Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
+                      StringRef Arg3) const {
   if (Diags.isDiagnosticInFlight())
-    Diags.SetDelayedDiagnostic(DiagID, Arg1, Arg2);
+    Diags.SetDelayedDiagnostic(DiagID, Arg1, Arg2, Arg3);
   else
-    Diag(DiagID) << Arg1 << Arg2;
+    Diag(DiagID) << Arg1 << Arg2 << Arg3;
 }
 
 void ASTReader::Error(unsigned DiagID, StringRef Arg1, StringRef Arg2,
                       unsigned Select) const {
   if (!Diags.isDiagnosticInFlight())
     Diag(DiagID) << Arg1 << Arg2 << Select;
 }
 
case SUBMODULE_DEFINITION: {
         return Failure;
       }
 
       if (!ParentModule) {
         if (const FileEntry *CurFile = CurrentModule->getASTFile()) {
           // Don't emit module relocation error if we have -fno-validate-pch
           if (!PP.getPreprocessorOpts().DisablePCHValidation &&
               CurFile != F.File) {
-            if (!Diags.isDiagnosticInFlight()) {
-              Diag(diag::err_module_file_conflict)
-                << CurrentModule->getTopLevelModuleName()
-                << CurFile->getName()
-                << F.File->getName();
-            }
+            Error(diag::err_module_file_conflict,
+                  CurrentModule->getTopLevelModuleName(), CurFile->getName(),
+                  F.File->getName());
             return Failure;
           }
         }
 
         CurrentModule->setASTFile(F.File);
         CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
       }