[cfe-commits] r142120 - in /cfe/trunk: include/clang/Frontend/TextDiagnostic.h lib/Frontend/TextDiagnostic.cpp test/Misc/macro-backtrace.c

Chandler Carruth chandlerc at gmail.com
Sun Oct 16 00:20:28 PDT 2011


Author: chandlerc
Date: Sun Oct 16 02:20:28 2011
New Revision: 142120

URL: http://llvm.org/viewvc/llvm-project?rev=142120&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Frontend/TextDiagnostic.h
    cfe/trunk/lib/Frontend/TextDiagnostic.cpp
    cfe/trunk/test/Misc/macro-backtrace.c

Modified: cfe/trunk/include/clang/Frontend/TextDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/TextDiagnostic.h?rev=142120&r1=142119&r2=142120&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/TextDiagnostic.h (original)
+++ cfe/trunk/include/clang/Frontend/TextDiagnostic.h Sun Oct 16 02:20:28 2011
@@ -127,12 +127,13 @@
   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);
 

Modified: cfe/trunk/lib/Frontend/TextDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnostic.cpp?rev=142120&r1=142119&r2=142120&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/TextDiagnostic.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnostic.cpp Sun Oct 16 02:20:28 2011
@@ -420,15 +420,8 @@
                          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 @@
         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 @@
   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 @@
   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 @@
   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 @@
     }
     OS << "note: expanded from:\n";
 
-    emitSnippetAndCaret(Loc, Ranges, ArrayRef<FixItHint>());
+    emitSnippetAndCaret(Loc, DiagnosticsEngine::Note, Ranges,
+                        ArrayRef<FixItHint>());
     return;
   }
 
@@ -761,12 +757,24 @@
 /// \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;

Modified: cfe/trunk/test/Misc/macro-backtrace.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/macro-backtrace.c?rev=142120&r1=142119&r2=142120&view=diff
==============================================================================
--- cfe/trunk/test/Misc/macro-backtrace.c (original)
+++ cfe/trunk/test/Misc/macro-backtrace.c Sun Oct 16 02:20:28 2011
@@ -31,4 +31,17 @@
   // 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:
 }





More information about the cfe-commits mailing list