Rationalize some of how the locations of prior diagnostics are tracked

across emissions.

1) The include stack printing is conditioned on non-note diagnostics,
   not just on warning diagnostics.
2) Those should be full source locations as they're tied to a source
   manager.
3) We should pass in the prior state to the TextDiagnostic constructor,
   allow it to mutate as diagnostics are emitted, and then cache the
   final state before tearing it down.

Some of this remains incomplete, specifically #3 isn't finished for the
non-note location. That'll come when the include stack printing sinks
down a level.

This also highlights how *completely* bug-ridden this code is. For
example, we currently do all these comparisons of a FullSourceLoc and
a SourceLocation... which silently does a SourceLocation to
SourceLocation comparison, completely disregarding the source manager
from whence one of the arguments came. Oops! Good thing in practice this
wasn't important, but it could in theory be suppressing caret
diagnostics in a second TU on a single clang invocation. I'm hoping to
hammer these bugs out as the refactorings occur, although for so many of
them it's really unlikely I can dream up a test case that would show the
potentially buggy behavior.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@142067 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Frontend/TextDiagnosticPrinter.h b/include/clang/Frontend/TextDiagnosticPrinter.h
index 73971a073775348b59713f0ec19beef6f11e7c31..80fffd168458f0ff7e6acd2ea8bcfca0271936a4 100644 (file)
--- a/include/clang/Frontend/TextDiagnosticPrinter.h
+++ b/include/clang/Frontend/TextDiagnosticPrinter.h
@@ -27,8 +27,8 @@ class TextDiagnosticPrinter : public DiagnosticConsumer {
   const LangOptions *LangOpts;
   const DiagnosticOptions *DiagOpts;
 
-  SourceLocation LastWarningLoc;
   FullSourceLoc LastLoc;
+  FullSourceLoc LastNonNoteLoc;
   unsigned LastCaretDiagnosticWasNote : 1;
   unsigned OwnsOutputStream : 1;
 

diff --git a/lib/Frontend/TextDiagnosticPrinter.cpp b/lib/Frontend/TextDiagnosticPrinter.cpp
index 62ce6843877955a5e9a3674d46820f446ebc0858..8593e22f101f8c5859b17154450daab8fdc808c3 100644 (file)
--- a/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -91,9 +91,9 @@ void TextDiagnosticPrinter::PrintIncludeStack(DiagnosticsEngine::Level Level,
                                               SourceLocation Loc,
                                               const SourceManager &SM) {
   // Skip redundant include stacks altogether.
-  if (LastWarningLoc == Loc)
+  if (LastNonNoteLoc == Loc)
     return;
-  LastWarningLoc = Loc;
+  LastNonNoteLoc = FullSourceLoc(Loc, SM);
 
   if (!DiagOpts->ShowNoteIncludeStack && Level == DiagnosticsEngine::Note)
     return;
@@ -537,19 +537,40 @@ class TextDiagnostic {
   const LangOptions &LangOpts;
   const DiagnosticOptions &DiagOpts;
 
+  /// \brief The location of the previous diagnostic if known.
+  ///
+  /// This will be invalid in cases where there is no (known) previous
+  /// diagnostic location, or that location itself is invalid or comes from
+  /// a different source manager than SM.
+  SourceLocation LastLoc;
+
+  /// \brief The location of the previous non-note diagnostic if known.
+  ///
+  /// Same restriction as \see LastLoc but tracks the last non-note location.
+  SourceLocation LastNonNoteLoc;
+
 public:
   TextDiagnostic(TextDiagnosticPrinter &Printer,
-                  raw_ostream &OS,
-                  const SourceManager &SM,
-                  const LangOptions &LangOpts,
-                  const DiagnosticOptions &DiagOpts)
-    : Printer(Printer), OS(OS), SM(SM), LangOpts(LangOpts), DiagOpts(DiagOpts) {
+                 raw_ostream &OS,
+                 const SourceManager &SM,
+                 const LangOptions &LangOpts,
+                 const DiagnosticOptions &DiagOpts,
+                 FullSourceLoc LastLoc = FullSourceLoc(),
+                 FullSourceLoc LastNonNoteLoc = FullSourceLoc())
+    : Printer(Printer), OS(OS), SM(SM), LangOpts(LangOpts), DiagOpts(DiagOpts),
+      LastLoc(LastLoc), LastNonNoteLoc(LastNonNoteLoc) {
+    if (LastLoc.isValid() && &SM != &LastLoc.getManager())
+      this->LastLoc = SourceLocation();
+    if (LastNonNoteLoc.isValid() && &SM != &LastNonNoteLoc.getManager())
+      this->LastNonNoteLoc = SourceLocation();
   }
 
+  /// \brief Get the last diagnostic location emitted.
+  SourceLocation getLastLoc() const { return LastLoc; }
+
   void Emit(SourceLocation Loc, DiagnosticsEngine::Level Level,
             StringRef Message, ArrayRef<CharSourceRange> Ranges,
             ArrayRef<FixItHint> FixItHints,
-            FullSourceLoc LastLoc = FullSourceLoc(),
             bool LastCaretDiagnosticWasNote = false) {
     PresumedLoc PLoc = getDiagnosticPresumedLoc(SM, Loc);
 
@@ -592,6 +613,8 @@ public:
       unsigned MacroDepth = 0;
       EmitCaret(Loc, MutableRanges, FixItHints, MacroDepth);
     }
+
+    LastLoc = Loc;
   }
 
   /// \brief Emit the caret and underlining text.
@@ -1262,16 +1285,17 @@ void TextDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level,
   assert(Info.hasSourceManager() &&
          "Unexpected diagnostic with no source manager");
   const SourceManager &SM = Info.getSourceManager();
-  TextDiagnostic TextDiag(*this, OS, SM, *LangOpts, *DiagOpts);
+  TextDiagnostic TextDiag(*this, OS, SM, *LangOpts, *DiagOpts,
+                          LastNonNoteLoc, LastLoc);
 
   TextDiag.Emit(Info.getLocation(), Level, DiagMessageStream.str(),
                 Info.getRanges(),
                 llvm::makeArrayRef(Info.getFixItHints(),
                                    Info.getNumFixItHints()),
-                LastLoc, LastCaretDiagnosticWasNote);
+                LastCaretDiagnosticWasNote);
 
-  // Cache the LastLoc, it allows us to omit duplicate source/caret spewage.
-  LastLoc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+  // Cache the LastLoc from the TextDiagnostic printing.
+  LastLoc = FullSourceLoc(TextDiag.getLastLoc(), SM);
   LastCaretDiagnosticWasNote = (Level == DiagnosticsEngine::Note);
 
   OS.flush();