[cfe-commits] r142120 - in /cfe/trunk: include/clang/Frontend/TextDiagnostic.h lib/Frontend/TextDiagnostic.cpp test/Misc/macro-backtrace.c
Eli Friedman
eli.friedman at gmail.com
Mon Oct 17 20:09:33 PDT 2011
On Sun, Oct 16, 2011 at 12:20 AM, Chandler Carruth <chandlerc at gmail.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list