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.

llvm-svn: 142120
This commit is contained in:
Chandler Carruth 2011-10-16 07:20:28 +00:00
parent 5371981593
commit dc2f257680
3 changed files with 55 additions and 33 deletions

View File

@ -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);

View File

@ -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.
/// \brief Recursively emit notes for each macro expansion and caret
/// diagnostics where appropriate.
///
/// 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.
///
/// 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;

View File

@ -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:
}