Introduce code modification hints into the diagnostics system. When we
know how to recover from an error, we can attach a hint to the
diagnostic that states how to modify the code, which can be one of:
- Insert some new code (a text string) at a particular source
location
- Remove the code within a given range
- Replace the code within a given range with some new code (a text
string)
Right now, we use these hints to annotate diagnostic information. For
example, if one uses the '>>' in a template argument in C++98, as in
this code:
template<int I> class B { };
B<1000 >> 2> *b1;
we'll warn that the behavior will change in C++0x. The fix is to
insert parenthese, so we use code insertion annotations to illustrate
where the parentheses go:
test.cpp:10:10: warning: use of right-shift operator ('>>') in template
argument will require parentheses in C++0x
B<1000 >> 2> *b1;
^
( )
Use of these annotations is partially implemented for HTML
diagnostics, but it's not (yet) producing valid HTML, which may be
related to PR2386, so it has been #if 0'd out.
In this future, we could consider hooking this mechanism up to the
rewriter to actually try to fix these problems during compilation (or,
after a compilation whose only errors have fixes). For now, however, I
suggest that we use these code modification hints whenever we can, so
that we get better diagnostics now and will have better coverage when
we find better ways to use this information.
This also fixes PR3410 by placing the complaint about missing tokens
just after the previous token (rather than at the location of the next
token).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@65570 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Analysis/PathDiagnostic.cpp b/lib/Analysis/PathDiagnostic.cpp
index 4fa88ed..50ff523 100644
--- a/lib/Analysis/PathDiagnostic.cpp
+++ b/lib/Analysis/PathDiagnostic.cpp
@@ -47,6 +47,8 @@
for (unsigned i = 0, e = Info.getNumRanges(); i != e; ++i)
P->addRange(Info.getRange(i));
+ for (unsigned i = 0, e = Info.getNumCodeModificationHints(); i != e; ++i)
+ P->addCodeModificationHint(Info.getCodeModificationHint(i));
D->push_front(P);
HandlePathDiagnostic(D);
diff --git a/lib/Basic/TokenKinds.cpp b/lib/Basic/TokenKinds.cpp
index bde8a55..3d47863 100644
--- a/lib/Basic/TokenKinds.cpp
+++ b/lib/Basic/TokenKinds.cpp
@@ -27,3 +27,66 @@
assert(Kind < tok::NUM_TOKENS);
return TokNames[Kind];
}
+
+/// \brief Determines the spelling of simple punctuation tokens like
+/// '!' or '%', and returns NULL for literal and annotation tokens.
+const char *tok::getTokenSpelling(enum TokenKind Kind) {
+ switch (Kind) {
+ case tok::l_square: return "[";
+ case tok::r_square: return "]";
+ case tok::l_paren: return "(";
+ case tok::r_paren: return ")";
+ case tok::l_brace: return "{";
+ case tok::r_brace: return "}";
+ case tok::period: return ".";
+ case tok::ellipsis: return "...";
+ case tok::amp: return "&";
+ case tok::ampamp: return "&&";
+ case tok::ampequal: return "&=";
+ case tok::star: return "*";
+ case tok::starequal: return "*=";
+ case tok::plus: return "+";
+ case tok::plusplus: return "++";
+ case tok::plusequal: return "+=";
+ case tok::minus: return "-";
+ case tok::arrow: return "->";
+ case tok::minusminus: return "--";
+ case tok::minusequal: return "-=";
+ case tok::tilde: return "~";
+ case tok::exclaim: return "!";
+ case tok::exclaimequal: return "!=";
+ case tok::slash: return "/";
+ case tok::slashequal: return "/=";
+ case tok::percent: return "%";
+ case tok::percentequal: return "%=";
+ case tok::less: return "<";
+ case tok::lessless: return "<<";
+ case tok::lessequal: return "<=";
+ case tok::lesslessequal: return "<<=";
+ case tok::greater: return ">";
+ case tok::greatergreater: return ">>";
+ case tok::greaterequal: return ">=";
+ case tok::greatergreaterequal: return ">>=";
+ case tok::caret: return "^";
+ case tok::caretequal: return "^=";
+ case tok::pipe: return "|";
+ case tok::pipepipe: return "||";
+ case tok::pipeequal: return "|=";
+ case tok::question: return "?";
+ case tok::colon: return ":";
+ case tok::semi: return ";";
+ case tok::equal: return "=";
+ case tok::equalequal: return "==";
+ case tok::comma: return ",";
+ case tok::hash: return "#";
+ case tok::hashhash: return "##";
+ case tok::hashat: return "#@";
+ case tok::periodstar: return ".*";
+ case tok::arrowstar: return "->*";
+ case tok::coloncolon: return "::";
+ case tok::at: return "@";
+ default: break;
+ }
+
+ return 0;
+}
diff --git a/lib/Driver/HTMLDiagnostics.cpp b/lib/Driver/HTMLDiagnostics.cpp
index 6a4bf23..8ab9b18 100644
--- a/lib/Driver/HTMLDiagnostics.cpp
+++ b/lib/Driver/HTMLDiagnostics.cpp
@@ -51,7 +51,9 @@
void HandlePiece(Rewriter& R, FileID BugFileID,
const PathDiagnosticPiece& P, unsigned num, unsigned max);
- void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range);
+ void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range,
+ const char *HighlightStart = "<span class=\"mrange\">",
+ const char *HighlightEnd = "</span>");
void ReportDiag(const PathDiagnostic& D);
};
@@ -438,10 +440,33 @@
for (const SourceRange *I = P.ranges_begin(), *E = P.ranges_end();
I != E; ++I)
HighlightRange(R, LPosInfo.first, *I);
+
+#if 0
+ // If there is a code insertion hint, insert that code.
+ // FIXME: This code is disabled because it seems to mangle the HTML
+ // output. I'm leaving it here because it's generally the right idea,
+ // but needs some help from someone more familiar with the rewriter.
+ for (const CodeModificationHint *Hint = P.code_modifications_begin(),
+ *HintEnd = P.code_modifications_end();
+ Hint != HintEnd; ++Hint) {
+ if (Hint->RemoveRange.isValid()) {
+ HighlightRange(R, LPosInfo.first, Hint->RemoveRange,
+ "<span class=\"CodeRemovalHint\">", "</span>");
+ }
+ if (Hint->InsertionLoc.isValid()) {
+ std::string EscapedCode = html::EscapeText(Hint->CodeToInsert, true);
+ EscapedCode = "<span class=\"CodeInsertionHint\">" + EscapedCode
+ + "</span>";
+ R.InsertStrBefore(Hint->InsertionLoc, EscapedCode);
+ }
+ }
+#endif
}
void HTMLDiagnostics::HighlightRange(Rewriter& R, FileID BugFileID,
- SourceRange Range) {
+ SourceRange Range,
+ const char *HighlightStart,
+ const char *HighlightEnd) {
SourceManager& SM = R.getSourceMgr();
@@ -473,6 +498,5 @@
SourceLocation E =
InstantiationEnd.getFileLocWithOffset(EndColNo - OldEndColNo);
- html::HighlightRange(R, InstantiationStart, E,
- "<span class=\"mrange\">", "</span>");
+ html::HighlightRange(R, InstantiationStart, E, HighlightStart, HighlightEnd);
}
diff --git a/lib/Driver/TextDiagnosticPrinter.cpp b/lib/Driver/TextDiagnosticPrinter.cpp
index b5226f5..8a7e40f 100644
--- a/lib/Driver/TextDiagnosticPrinter.cpp
+++ b/lib/Driver/TextDiagnosticPrinter.cpp
@@ -16,6 +16,7 @@
#include "clang/Lex/Lexer.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/ADT/SmallString.h"
+#include <algorithm>
using namespace clang;
void TextDiagnosticPrinter::
@@ -104,7 +105,9 @@
void TextDiagnosticPrinter::EmitCaretDiagnostic(SourceLocation Loc,
SourceRange *Ranges,
unsigned NumRanges,
- SourceManager &SM) {
+ SourceManager &SM,
+ const CodeModificationHint *Hints,
+ unsigned NumHints) {
assert(!Loc.isInvalid() && "must have a valid source location here");
// We always emit diagnostics about the instantiation points, not the spelling
@@ -205,6 +208,36 @@
// Emit what we have computed.
OS << SourceLine << '\n';
OS << CaretLine << '\n';
+
+ if (NumHints) {
+ std::string InsertionLine;
+ for (const CodeModificationHint *Hint = Hints,
+ *LastHint = Hints + NumHints;
+ Hint != LastHint; ++Hint) {
+ if (Hint->InsertionLoc.isValid()) {
+ // We have an insertion hint. Determine whether the inserted
+ // code is on the same line as the caret.
+ std::pair<FileID, unsigned> HintLocInfo
+ = SM.getDecomposedLoc(Hint->InsertionLoc);
+ if (SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) ==
+ SM.getLineNumber(FID, FileOffset)) {
+ // Insert the new code into the line just below the code
+ // that the user wrote.
+ unsigned HintColNo
+ = SM.getColumnNumber(HintLocInfo.first, HintLocInfo.second);
+ unsigned LastColumnModified
+ = HintColNo - 1 + Hint->CodeToInsert.size();
+ if (LastColumnModified > InsertionLine.size())
+ InsertionLine.resize(LastColumnModified, ' ');
+ std::copy(Hint->CodeToInsert.begin(), Hint->CodeToInsert.end(),
+ InsertionLine.begin() + HintColNo - 1);
+ }
+ }
+ }
+
+ if (!InsertionLine.empty())
+ OS << InsertionLine << '\n';
+ }
}
@@ -252,18 +285,30 @@
// 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 (CaretDiagnostics && Info.getLocation().isValid() &&
- ((LastLoc != Info.getLocation()) || Info.getNumRanges())) {
+ ((LastLoc != Info.getLocation()) || Info.getNumRanges() ||
+ Info.getNumCodeModificationHints())) {
// Cache the LastLoc, it allows us to omit duplicate source/caret spewage.
LastLoc = Info.getLocation();
// Get the ranges into a local array we can hack on.
- SourceRange Ranges[10];
+ SourceRange Ranges[20];
unsigned NumRanges = Info.getNumRanges();
- assert(NumRanges < 10 && "Out of space");
+ assert(NumRanges < 20 && "Out of space");
for (unsigned i = 0; i != NumRanges; ++i)
Ranges[i] = Info.getRange(i);
- EmitCaretDiagnostic(LastLoc, Ranges, NumRanges, LastLoc.getManager());
+ unsigned NumHints = Info.getNumCodeModificationHints();
+ for (unsigned idx = 0; idx < NumHints; ++idx) {
+ const CodeModificationHint &Hint = Info.getCodeModificationHint(idx);
+ if (Hint.RemoveRange.isValid()) {
+ assert(NumRanges < 20 && "Out of space");
+ Ranges[NumRanges++] = Hint.RemoveRange;
+ }
+ }
+
+ EmitCaretDiagnostic(LastLoc, Ranges, NumRanges, LastLoc.getManager(),
+ Info.getCodeModificationHints(),
+ Info.getNumCodeModificationHints());
}
OS.flush();
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index d0808a5..6c6842e 100644
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -365,10 +365,19 @@
if (!LHS.isInvalid()) {
// Combine the LHS and RHS into the LHS (e.g. build AST).
- if (TernaryMiddle.isInvalid())
+ if (TernaryMiddle.isInvalid()) {
+ // If we're using '>>' as an operator within a template
+ // argument list (in C++98), suggest the addition of
+ // parentheses so that the code remains well-formed in C++0x.
+ if (!GreaterThanIsOperator && OpToken.is(tok::greatergreater))
+ SuggestParentheses(OpToken.getLocation(),
+ diag::warn_cxx0x_right_shift_in_template_arg,
+ SourceRange(Actions.getExprRange(LHS.get()).getBegin(),
+ Actions.getExprRange(RHS.get()).getEnd()));
+
LHS = Actions.ActOnBinOp(CurScope, OpToken.getLocation(),
OpToken.getKind(), move(LHS), move(RHS));
- else
+ } else
LHS = Actions.ActOnConditionalOp(OpToken.getLocation(), ColonLoc,
move(LHS), move(TernaryMiddle),
move(RHS));
diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp
index 65705e8..0209d7b 100644
--- a/lib/Parse/ParseTemplate.cpp
+++ b/lib/Parse/ParseTemplate.cpp
@@ -439,8 +439,14 @@
RAngleLoc = Tok.getLocation();
if (Tok.is(tok::greatergreater)) {
- if (!getLang().CPlusPlus0x)
- Diag(Tok.getLocation(), diag::err_two_right_angle_brackets_need_space);
+ if (!getLang().CPlusPlus0x) {
+ const char *ReplaceStr = "> >";
+ if (NextToken().is(tok::greater) || NextToken().is(tok::greatergreater))
+ ReplaceStr = "> > ";
+
+ Diag(Tok.getLocation(), diag::err_two_right_angle_brackets_need_space)
+ << CodeReplacementHint(SourceRange(Tok.getLocation()), ReplaceStr);
+ }
Tok.setKind(tok::greater);
if (!ConsumeLastToken) {
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index a3ed027..a67106c 100644
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -68,6 +68,28 @@
return Diag(Tok.getLocation(), DiagID);
}
+/// \brief Emits a diagnostic suggesting parentheses surrounding a
+/// given range.
+///
+/// \param Loc The location where we'll emit the diagnostic.
+/// \param Loc The kind of diagnostic to emit.
+/// \param ParenRange Source range enclosing code that should be parenthesized.
+void Parser::SuggestParentheses(SourceLocation Loc, unsigned DK,
+ SourceRange ParenRange) {
+ if (!ParenRange.getEnd().isFileID()) {
+ // We can't display the parentheses, so just dig the
+ // warning/error and return.
+ Diag(Loc, DK);
+ return;
+ }
+
+ unsigned Len = Lexer::MeasureTokenLength(ParenRange.getEnd(),
+ PP.getSourceManager());
+ Diag(Loc, DK)
+ << CodeInsertionHint(ParenRange.getBegin(), "(")
+ << CodeInsertionHint(ParenRange.getEnd().getFileLocWithOffset(Len), ")");
+}
+
/// MatchRHSPunctuation - For punctuation with a LHS and RHS (e.g. '['/']'),
/// this helper function matches and consumes the specified RHS token if
/// present. If not present, it emits the specified diagnostic indicating
@@ -108,7 +130,18 @@
return false;
}
- Diag(Tok, DiagID) << Msg;
+ const char *Spelling = 0;
+ if (PrevTokLocation.isValid() && PrevTokLocation.isFileID() &&
+ (Spelling = tok::getTokenSpelling(ExpectedTok))) {
+ // Show what code to insert to fix this problem.
+ SourceLocation DiagLoc
+ = PrevTokLocation.getFileLocWithOffset(strlen(Spelling));
+ Diag(DiagLoc, DiagID)
+ << Msg
+ << CodeInsertionHint(DiagLoc, Spelling);
+ } else
+ Diag(Tok, DiagID) << Msg;
+
if (SkipToTok != tok::unknown)
SkipUntil(SkipToTok);
return true;
diff --git a/lib/Rewrite/HTMLRewrite.cpp b/lib/Rewrite/HTMLRewrite.cpp
index 0ef998d..692af80 100644
--- a/lib/Rewrite/HTMLRewrite.cpp
+++ b/lib/Rewrite/HTMLRewrite.cpp
@@ -319,6 +319,9 @@
" .mrange { background-color:#dfddf3 }\n"
" .mrange { border-bottom:1px solid #6F9DBE }\n"
" .PathIndex { font-weight: bold }\n"
+ " .CodeInsertionHint { font-weight: bold; background-color: #10dd10 }\n"
+ " .CodeRemovalHint { background-color:#de1010 }\n"
+ " .CodeRemovalHint { border-bottom:1px solid #6F9DBE }\n"
" table.simpletable {\n"
" padding: 5px;\n"
" font-size:12pt;\n"
diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 25f2164..68e0ce8 100644
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -1036,6 +1036,8 @@
bool DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc);
// Primary Expressions.
+ virtual SourceRange getExprRange(ExprTy *E) const;
+
virtual OwningExprResult ActOnIdentifierExpr(Scope *S, SourceLocation Loc,
IdentifierInfo &II,
bool HasTrailingLParen,
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 1f8cb80..eec27ce 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -110,6 +110,11 @@
return false;
}
+SourceRange Sema::getExprRange(ExprTy *E) const {
+ Expr *Ex = (Expr *)E;
+ return Ex? Ex->getSourceRange() : SourceRange();
+}
+
//===----------------------------------------------------------------------===//
// Standard Promotions and Conversions
//===----------------------------------------------------------------------===//
diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp
index 0c60520..f69f546 100644
--- a/lib/Sema/SemaTemplate.cpp
+++ b/lib/Sema/SemaTemplate.cpp
@@ -1615,12 +1615,10 @@
// FIXME: Once we have member templates, we'll need to check
// C++ [temp.expl.spec]p17-18, where we could have multiple levels of
// template<> headers.
- if (TemplateParameterLists.size() == 0) {
- // FIXME: It would be very nifty if we could introduce some kind
- // of "code insertion hint" that could show the code that needs to
- // be added.
- Diag(KWLoc, diag::err_template_spec_needs_header);
- } else {
+ if (TemplateParameterLists.size() == 0)
+ Diag(KWLoc, diag::err_template_spec_needs_header)
+ << CodeInsertionHint(KWLoc, "template<> ");
+ else {
TemplateParameterList *TemplateParams
= static_cast<TemplateParameterList*>(*TemplateParameterLists.get());
if (TemplateParameterLists.size() > 1) {