On Sun, Oct 16, 2011 at 12:20 AM, Chandler Carruth <[email protected]> wrote: > 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.
Err, it turns out our infrastructure for running the gcc testsuite over clang was depending on this side-effect of -fno-caret-diagnostics; mind adding a flag to turn off macro backtraces? -Eli > 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: > } > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
