Now that the structure of this is more reasonably laid out, fix a long

standing deficiency: we were providing no macro backtrace information
whenever caret diagnostics were turned off. This sinks the logic for
suppressing the code snippet and caret to the code that actually prints
tho code snippet and caret. Along the way, clean up the naming of
functions, remove some now fixed FIXMEs, and generally improve the
wording and logic of this process.

Add a test case exerecising this functionality. It is notable that the
resulting messages are extremely low quality. I'm working on a follow-up
patch that should address this and have left a FIXME in the test case.

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

diff --git a/include/clang/Frontend/TextDiagnostic.h b/include/clang/Frontend/TextDiagnostic.h
index 84e23b1b337be37df4a9833700ad81872ae05d57..ce86914bca7bbed97ce376c5a37d6c29a4b92e6b 100644 (file)
--- a/include/clang/Frontend/TextDiagnostic.h
+++ b/include/clang/Frontend/TextDiagnostic.h
@@ -127,12 +127,13 @@ private:
   void emitDiagnosticLoc(SourceLocation Loc, PresumedLoc PLoc,
                          DiagnosticsEngine::Level Level,
                          ArrayRef<CharSourceRange> Ranges);
-  void emitCaret(SourceLocation Loc,
-                 SmallVectorImpl<CharSourceRange>& Ranges,
-                 ArrayRef<FixItHint> Hints,
-                 unsigned &MacroDepth,
-                 unsigned OnMacroInst = 0);
-  void emitSnippetAndCaret(SourceLocation Loc,
+  void emitMacroExpansionsAndCarets(SourceLocation Loc,
+                                    DiagnosticsEngine::Level Level,
+                                    SmallVectorImpl<CharSourceRange>& Ranges,
+                                    ArrayRef<FixItHint> Hints,
+                                    unsigned &MacroDepth,
+                                    unsigned OnMacroInst = 0);
+  void emitSnippetAndCaret(SourceLocation Loc, DiagnosticsEngine::Level Level,
                            SmallVectorImpl<CharSourceRange>& Ranges,
                            ArrayRef<FixItHint> Hints);
 

diff --git a/lib/Frontend/TextDiagnostic.cpp b/lib/Frontend/TextDiagnostic.cpp
index 9bae0dff938a5ea073967562ff4e85bc10bce0be..346443d109401e0b9c50d48e53deb907d5ca98b1 100644 (file)
--- a/lib/Frontend/TextDiagnostic.cpp
+++ b/lib/Frontend/TextDiagnostic.cpp
@@ -420,15 +420,8 @@ void TextDiagnostic::emitDiagnostic(SourceLocation Loc,
                          OS.tell() - StartOfLocationInfo,
                          DiagOpts.MessageLength, DiagOpts.ShowColors);
 
-  // If caret diagnostics are enabled and we have location, we want to
-  // emit the caret.  However, we only do this if the location moved
-  // from the last diagnostic, if the last diagnostic was a note that
-  // was part of a different warning or error diagnostic, or if the
-  // diagnostic has ranges.  We don't want to emit the same caret
-  // multiple times if one loc has multiple diagnostics.
-  if (DiagOpts.ShowCarets &&
-      (Loc != LastLoc || !Ranges.empty() || !FixItHints.empty() ||
-       (LastLevel == DiagnosticsEngine::Note && Level != LastLevel))) {
+  // Only recurse if we have a valid location.
+  if (Loc.isValid()) {
     // Get the ranges into a local array we can hack on.
     SmallVector<CharSourceRange, 20> MutableRanges(Ranges.begin(),
                                                    Ranges.end());
@@ -440,7 +433,8 @@ void TextDiagnostic::emitDiagnostic(SourceLocation Loc,
         MutableRanges.push_back(I->RemoveRange);
 
     unsigned MacroDepth = 0;
-    emitCaret(Loc, MutableRanges, FixItHints, MacroDepth);
+    emitMacroExpansionsAndCarets(Loc, Level, MutableRanges, FixItHints,
+                                 MacroDepth);
   }
 
   LastLoc = Loc;
@@ -653,26 +647,26 @@ void TextDiagnostic::emitDiagnosticLoc(SourceLocation Loc, PresumedLoc PLoc,
   OS << ' ';
 }
 
-/// \brief Emit the caret and underlining text.
-///
-/// Walks up the macro expansion stack printing the code snippet, caret,
-/// underlines and FixItHint display as appropriate at each level. Walk is
-/// accomplished by calling itself recursively.
+/// \brief Recursively emit notes for each macro expansion and caret
+/// diagnostics where appropriate.
 ///
-/// FIXME: Remove macro expansion from this routine, it shouldn't be tied to
-/// caret diagnostics.
-/// FIXME: Break up massive function into logical units.
+/// Walks up the macro expansion stack printing expansion notes, the code
+/// snippet, caret, underlines and FixItHint display as appropriate at each
+/// level.
 ///
 /// \param Loc The location for this caret.
+/// \param Level The diagnostic level currently being emitted.
 /// \param Ranges The underlined ranges for this code snippet.
 /// \param Hints The FixIt hints active for this diagnostic.
 /// \param MacroSkipEnd The depth to stop skipping macro expansions.
 /// \param OnMacroInst The current depth of the macro expansion stack.
-void TextDiagnostic::emitCaret(SourceLocation Loc,
-                               SmallVectorImpl<CharSourceRange>& Ranges,
-                               ArrayRef<FixItHint> Hints,
-                               unsigned &MacroDepth,
-                               unsigned OnMacroInst) {
+void TextDiagnostic::emitMacroExpansionsAndCarets(
+    SourceLocation Loc,
+    DiagnosticsEngine::Level Level,
+    SmallVectorImpl<CharSourceRange>& Ranges,
+    ArrayRef<FixItHint> Hints,
+    unsigned &MacroDepth,
+    unsigned OnMacroInst) {
   assert(!Loc.isInvalid() && "must have a valid source location here");
 
   // If this is a file source location, directly emit the source snippet and
@@ -680,7 +674,7 @@ void TextDiagnostic::emitCaret(SourceLocation Loc,
   if (Loc.isFileID()) {
     assert(MacroDepth == 0 && "We shouldn't hit a leaf node twice!");
     MacroDepth = OnMacroInst;
-    emitSnippetAndCaret(Loc, Ranges, Hints);
+    emitSnippetAndCaret(Loc, Level, Ranges, Hints);
     return;
   }
   // Otherwise recurse through each macro expansion layer.
@@ -692,7 +686,8 @@ void TextDiagnostic::emitCaret(SourceLocation Loc,
   SourceLocation OneLevelUp = getImmediateMacroCallerLoc(SM, Loc);
 
   // FIXME: Map ranges?
-  emitCaret(OneLevelUp, Ranges, Hints, MacroDepth, OnMacroInst + 1);
+  emitMacroExpansionsAndCarets(OneLevelUp, Level, Ranges, Hints, MacroDepth,
+                               OnMacroInst + 1);
 
   // Map the location.
   Loc = getImmediateMacroCalleeLoc(SM, Loc);
@@ -741,7 +736,8 @@ void TextDiagnostic::emitCaret(SourceLocation Loc,
     }
     OS << "note: expanded from:\n";
 
-    emitSnippetAndCaret(Loc, Ranges, ArrayRef<FixItHint>());
+    emitSnippetAndCaret(Loc, DiagnosticsEngine::Note, Ranges,
+                        ArrayRef<FixItHint>());
     return;
   }
 
@@ -761,12 +757,24 @@ void TextDiagnostic::emitCaret(SourceLocation Loc,
 /// \param Ranges The underlined ranges for this code snippet.
 /// \param Hints The FixIt hints active for this diagnostic.
 void TextDiagnostic::emitSnippetAndCaret(
-    SourceLocation Loc,
+    SourceLocation Loc, DiagnosticsEngine::Level Level,
     SmallVectorImpl<CharSourceRange>& Ranges,
     ArrayRef<FixItHint> Hints) {
   assert(!Loc.isInvalid() && "must have a valid source location here");
   assert(Loc.isFileID() && "must have a file location here");
 
+  // If caret diagnostics are enabled and we have location, we want to
+  // emit the caret.  However, we only do this if the location moved
+  // from the last diagnostic, if the last diagnostic was a note that
+  // was part of a different warning or error diagnostic, or if the
+  // diagnostic has ranges.  We don't want to emit the same caret
+  // multiple times if one loc has multiple diagnostics.
+  if (!DiagOpts.ShowCarets)
+    return;
+  if (Loc == LastLoc && Ranges.empty() && Hints.empty() &&
+      (LastLevel != DiagnosticsEngine::Note || Level == LastLevel))
+    return;
+
   // Decompose the location into a FID/Offset pair.
   std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
   FileID FID = LocInfo.first;

diff --git a/test/Misc/macro-backtrace.c b/test/Misc/macro-backtrace.c
index 819ec93117967e4028515cf1045cd05c5435dfed..a76d4d22e381da72396d53acad418fb3fd8f0fac 100644 (file)
--- a/test/Misc/macro-backtrace.c
+++ b/test/Misc/macro-backtrace.c
@@ -31,4 +31,17 @@ void f(int *ip, float *fp) {
   // CHECK-LIMIT: #define M2(A, B) M1(A, B)
   // CHECK-LIMIT: macro-backtrace.c:4:23: note: expanded from:
   // CHECK-LIMIT: #define M1(A, B) ((A) < (B))
+
+  // FIXME: We should have higher quality messages, especially when caret
+  // diagnostics are off.
+  // RUN: %clang_cc1 -fsyntax-only -fno-caret-diagnostics %s 2>&1 \
+  // RUN:   | FileCheck %s -check-prefix=CHECK-NO-CARETS
+  // CHECK-NO-CARETS: macro-backtrace.c:18:7: warning: comparison of distinct pointer types ('int *' and 'float *')
+  // CHECK-NO-CARETS-NEXT: macro-backtrace.c:15:19: note: expanded from:
+  // CHECK-NO-CARETS-NEXT: macro-backtrace.c:14:19: note: expanded from:
+  // CHECK-NO-CARETS-NEXT: macro-backtrace.c:13:19: note: expanded from:
+  // CHECK-NO-CARETS-NEXT: note: (skipping 6 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
+  // CHECK-NO-CARETS-NEXT: macro-backtrace.c:6:18: note: expanded from:
+  // CHECK-NO-CARETS-NEXT: macro-backtrace.c:5:18: note: expanded from:
+  // CHECK-NO-CARETS-NEXT: macro-backtrace.c:4:23: note: expanded from:
 }