From 4b2d3f7bcc4df31157df443af1b80bcaa9b58bba Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Thu, 26 Feb 2009 21:00:50 +0000 Subject: [PATCH] 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 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 --- include/clang/Analysis/PathDiagnostic.h | 18 ++- include/clang/Basic/Diagnostic.h | 107 +++++++++++++++++- include/clang/Basic/DiagnosticParseKinds.def | 3 + include/clang/Basic/TokenKinds.h | 1 + include/clang/Driver/TextDiagnosticPrinter.h | 4 +- include/clang/Parse/Action.h | 6 + include/clang/Parse/Parser.h | 29 +++-- lib/Analysis/PathDiagnostic.cpp | 2 + lib/Basic/TokenKinds.cpp | 63 +++++++++++ lib/Driver/HTMLDiagnostics.cpp | 32 +++++- lib/Driver/TextDiagnosticPrinter.cpp | 55 ++++++++- lib/Parse/ParseExpr.cpp | 13 ++- lib/Parse/ParseTemplate.cpp | 10 +- lib/Parse/Parser.cpp | 35 +++++- lib/Rewrite/HTMLRewrite.cpp | 3 + lib/Sema/Sema.h | 2 + lib/Sema/SemaExpr.cpp | 5 + lib/Sema/SemaTemplate.cpp | 10 +- test/Parser/cxx-using-directive.cpp | 5 +- test/Parser/recovery.c | 5 + test/SemaTemplate/right-angle-brackets-98.cpp | 3 +- test/SemaTemplate/temp_arg_nontype.cpp | 2 +- 22 files changed, 371 insertions(+), 42 deletions(-) diff --git a/include/clang/Analysis/PathDiagnostic.h b/include/clang/Analysis/PathDiagnostic.h index bc68ac5911..cee11c27b5 100644 --- a/include/clang/Analysis/PathDiagnostic.h +++ b/include/clang/Analysis/PathDiagnostic.h @@ -31,6 +31,7 @@ public: private: FullSourceLoc Pos; std::string str; + std::vector CodeModificationHints; DisplayHint Hint; std::vector ranges; @@ -56,6 +57,10 @@ public: ranges.push_back(SourceRange(B,E)); } + void addCodeModificationHint(const CodeModificationHint& Hint) { + CodeModificationHints.push_back(Hint); + } + typedef const SourceRange* range_iterator; range_iterator ranges_begin() const { @@ -65,7 +70,18 @@ public: range_iterator ranges_end() const { return ranges_begin() + ranges.size(); } - + + typedef const CodeModificationHint *code_modifications_iterator; + + code_modifications_iterator code_modifications_begin() const { + return CodeModificationHints.empty()? 0 : &CodeModificationHints[0]; + } + + code_modifications_iterator code_modifications_end() const { + return CodeModificationHints.empty()? 0 + : &CodeModificationHints[0] + CodeModificationHints.size(); + } + const SourceManager& getSourceManager() const { return Pos.getManager(); } diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index db62ca95e7..7204abbd2a 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -68,6 +68,69 @@ namespace clang { }; } +/// \brief Annotates a diagnostic with some code that should be +/// inserted, removed, or replaced to fix the problem. +/// +/// This kind of hint should be used when we are certain that the +/// introduction, removal, or modification of a particular (small!) +/// amount of code will correct a compilation error. The compiler +/// should also provide full recovery from such errors, such that +/// suppressing the diagnostic output can still result successful +/// compilation. +class CodeModificationHint { +public: + /// \brief Tokens that should be removed to correct the error. + SourceRange RemoveRange; + + /// \brief The location at which we should insert code to correct + /// the error. + SourceLocation InsertionLoc; + + /// \brief The actual code to insert at the insertion location, as a + /// string. + std::string CodeToInsert; + + /// \brief Empty code modification hint, indicating that no code + /// modification is known. + CodeModificationHint() : RemoveRange(), InsertionLoc() { } + + /// \brief Create a code modification hint that inserts the given + /// code string at a specific location. + CodeModificationHint(SourceLocation InsertionLoc, const std::string &Code) + : RemoveRange(), InsertionLoc(InsertionLoc), CodeToInsert(Code) { } + + /// \brief Create a code modification hint that removes the given + /// source range. + CodeModificationHint(SourceRange RemoveRange) + : RemoveRange(RemoveRange), InsertionLoc(), CodeToInsert() { } + + /// \brief Create a code modification hint that replaces the given + /// source range with the given code string. + CodeModificationHint(SourceRange RemoveRange, const std::string &Code) + : RemoveRange(RemoveRange), InsertionLoc(RemoveRange.getBegin()), + CodeToInsert(Code) { } +}; + +/// \brief Creates a code modification hint that inserts the given +/// string at a particular location in the source code. +inline CodeModificationHint +CodeInsertionHint(SourceLocation InsertionLoc, const std::string &Code) { + return CodeModificationHint(InsertionLoc, Code); +} + +/// \brief Creates a code modification hint that removes the given +/// source range. +inline CodeModificationHint CodeRemovalHint(SourceRange RemoveRange) { + return CodeModificationHint(RemoveRange); +} + +/// \brief Creates a code modification hint that replaces the given +/// source range with the given code string. +inline CodeModificationHint +CodeReplacementHint(SourceRange RemoveRange, const std::string &Code) { + return CodeModificationHint(RemoveRange, Code); +} + /// Diagnostic - This concrete class is used by the front-end to report /// problems and issues. It massages the diagnostics (e.g. handling things like /// "report warnings as errors" and passes them off to the DiagnosticClient for @@ -272,6 +335,9 @@ private: signed char NumDiagArgs; /// NumRanges - This is the number of ranges in the DiagRanges array. unsigned char NumDiagRanges; + /// \brief The number of code modifications hints in the + /// CodeModificationHints array. + unsigned char NumCodeModificationHints; /// DiagArgumentsKind - This is an array of ArgumentKind::ArgumentKind enum /// values, with one for each argument. This specifies whether the argument @@ -293,6 +359,12 @@ private: /// only support 10 ranges, could easily be extended if needed. const SourceRange *DiagRanges[10]; + enum { MaxCodeModificationHints = 3 }; + + /// CodeModificationHints - If valid, provides a hint with some code + /// to insert, remove, or modify at a particular position. + CodeModificationHint CodeModificationHints[MaxCodeModificationHints]; + /// ProcessDiag - This is the method used to report a diagnostic that is /// finally fully formed. void ProcessDiag(); @@ -315,12 +387,13 @@ private: /// for example. class DiagnosticBuilder { mutable Diagnostic *DiagObj; - mutable unsigned NumArgs, NumRanges; + mutable unsigned NumArgs, NumRanges, NumCodeModificationHints; void operator=(const DiagnosticBuilder&); // DO NOT IMPLEMENT friend class Diagnostic; explicit DiagnosticBuilder(Diagnostic *diagObj) - : DiagObj(diagObj), NumArgs(0), NumRanges(0) {} + : DiagObj(diagObj), NumArgs(0), NumRanges(0), + NumCodeModificationHints(0) {} public: /// Copy constructor. When copied, this "takes" the diagnostic info from the @@ -339,6 +412,7 @@ public: // the Diagnostic object. DiagObj->NumDiagArgs = NumArgs; DiagObj->NumDiagRanges = NumRanges; + DiagObj->NumCodeModificationHints = NumCodeModificationHints; // Process the diagnostic, sending the accumulated information to the // DiagnosticClient. @@ -373,6 +447,12 @@ public: "Too many arguments to diagnostic!"); DiagObj->DiagRanges[NumRanges++] = &R; } + + void AddCodeModificationHint(const CodeModificationHint &Hint) const { + assert(NumCodeModificationHints < Diagnostic::MaxCodeModificationHints && + "Too many code modification hints!"); + DiagObj->CodeModificationHints[NumCodeModificationHints++] = Hint; + } }; inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, @@ -416,6 +496,12 @@ inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, DB.AddSourceRange(R); return DB; } + +inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, + const CodeModificationHint &Hint) { + DB.AddCodeModificationHint(Hint); + return DB; +} /// Report - Issue the message to the client. DiagID is a member of the @@ -433,7 +519,7 @@ inline DiagnosticBuilder Diagnostic::Report(FullSourceLoc Loc, unsigned DiagID){ //===----------------------------------------------------------------------===// /// DiagnosticInfo - This is a little helper class (which is basically a smart -/// pointer that forward info from Diagnostic) that allows clients ot enquire +/// pointer that forward info from Diagnostic) that allows clients to enquire /// about the currently in-flight diagnostic. class DiagnosticInfo { const Diagnostic *DiagObj; @@ -507,14 +593,25 @@ public: return *DiagObj->DiagRanges[Idx]; } - + unsigned getNumCodeModificationHints() const { + return DiagObj->NumCodeModificationHints; + } + + const CodeModificationHint &getCodeModificationHint(unsigned Idx) const { + return DiagObj->CodeModificationHints[Idx]; + } + + const CodeModificationHint *getCodeModificationHints() const { + return DiagObj->NumCodeModificationHints? + &DiagObj->CodeModificationHints[0] : 0; + } + /// FormatDiagnostic - Format this diagnostic into a string, substituting the /// formal arguments into the %0 slots. The result is appended onto the Str /// array. void FormatDiagnostic(llvm::SmallVectorImpl &OutStr) const; }; - /// DiagnosticClient - This is an abstract interface implemented by clients of /// the front-end, which formats and prints fully processed diagnostics. class DiagnosticClient { diff --git a/include/clang/Basic/DiagnosticParseKinds.def b/include/clang/Basic/DiagnosticParseKinds.def index 03868cda49..703d6b875c 100644 --- a/include/clang/Basic/DiagnosticParseKinds.def +++ b/include/clang/Basic/DiagnosticParseKinds.def @@ -280,6 +280,9 @@ DIAG(err_less_after_template_name_in_nested_name_spec, ERROR, "expected '<' after 'template %0' in nested name specifier") DIAG(err_two_right_angle_brackets_need_space, ERROR, "a space is required between consecutive right angle brackets (use '> >')") +DIAG(warn_cxx0x_right_shift_in_template_arg, WARNING, + "use of right-shift operator ('>>') in template argument will require " + "parentheses in C++0x") // Language specific pragmas diff --git a/include/clang/Basic/TokenKinds.h b/include/clang/Basic/TokenKinds.h index cfef30b4f5..6a14244362 100644 --- a/include/clang/Basic/TokenKinds.h +++ b/include/clang/Basic/TokenKinds.h @@ -44,6 +44,7 @@ enum ObjCKeywordKind { }; const char *getTokenName(enum TokenKind Kind); +const char *getTokenSpelling(enum TokenKind Kind); } // end namespace tok } // end namespace clang diff --git a/include/clang/Driver/TextDiagnosticPrinter.h b/include/clang/Driver/TextDiagnosticPrinter.h index 8d702b4934..4a332b28a8 100644 --- a/include/clang/Driver/TextDiagnosticPrinter.h +++ b/include/clang/Driver/TextDiagnosticPrinter.h @@ -48,7 +48,9 @@ public: void EmitCaretDiagnostic(SourceLocation Loc, SourceRange *Ranges, unsigned NumRanges, - SourceManager &SM); + SourceManager &SM, + const CodeModificationHint *Hints = 0, + unsigned NumHints = 0); virtual void HandleDiagnostic(Diagnostic::Level DiagLevel, const DiagnosticInfo &Info); diff --git a/include/clang/Parse/Action.h b/include/clang/Parse/Action.h index ebcd79caae..a7f2e767b3 100644 --- a/include/clang/Parse/Action.h +++ b/include/clang/Parse/Action.h @@ -551,6 +551,12 @@ public: // Primary Expressions. + /// \brief Retrieve the source range that corresponds to the given + /// expression. + virtual SourceRange getExprRange(ExprTy *E) const { + return SourceRange(); + } + /// ActOnIdentifierExpr - Parse an identifier in expression context. /// 'HasTrailingLParen' indicates whether or not the identifier has a '(' /// token immediately after it. diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 1e3ea317da..5d5265df35 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -37,6 +37,12 @@ class Parser { /// that this is valid. Token Tok; + // PrevTokLocation - The location of the token we previously + // consumed. This token is used for diagnostics where we expected to + // see a token following another token (e.g., the ';' at the end of + // a statement). + SourceLocation PrevTokLocation; + unsigned short ParenCount, BracketCount, BraceCount; /// Actions - These are the callbacks we invoke as we parse various constructs @@ -175,9 +181,9 @@ private: assert(!isTokenStringLiteral() && !isTokenParen() && !isTokenBracket() && !isTokenBrace() && "Should consume special tokens with Consume*Token"); - SourceLocation L = Tok.getLocation(); + PrevTokLocation = Tok.getLocation(); PP.Lex(Tok); - return L; + return PrevTokLocation; } /// ConsumeAnyToken - Dispatch to the right Consume* method based on the @@ -204,9 +210,9 @@ private: ++ParenCount; else if (ParenCount) --ParenCount; // Don't let unbalanced )'s drive the count negative. - SourceLocation L = Tok.getLocation(); + PrevTokLocation = Tok.getLocation(); PP.Lex(Tok); - return L; + return PrevTokLocation; } /// ConsumeBracket - This consume method keeps the bracket count up-to-date. @@ -218,9 +224,9 @@ private: else if (BracketCount) --BracketCount; // Don't let unbalanced ]'s drive the count negative. - SourceLocation L = Tok.getLocation(); + PrevTokLocation = Tok.getLocation(); PP.Lex(Tok); - return L; + return PrevTokLocation; } /// ConsumeBrace - This consume method keeps the brace count up-to-date. @@ -232,9 +238,9 @@ private: else if (BraceCount) --BraceCount; // Don't let unbalanced }'s drive the count negative. - SourceLocation L = Tok.getLocation(); + PrevTokLocation = Tok.getLocation(); PP.Lex(Tok); - return L; + return PrevTokLocation; } /// ConsumeStringToken - Consume the current 'peek token', lexing a new one @@ -244,9 +250,9 @@ private: SourceLocation ConsumeStringToken() { assert(isTokenStringLiteral() && "Should only consume string literals with this method"); - SourceLocation L = Tok.getLocation(); + PrevTokLocation = Tok.getLocation(); PP.Lex(Tok); - return L; + return PrevTokLocation; } /// GetLookAheadToken - This peeks ahead N tokens and returns that token @@ -399,6 +405,9 @@ private: DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID); DiagnosticBuilder Diag(const Token &Tok, unsigned DiagID); + void SuggestParentheses(SourceLocation Loc, unsigned DK, + SourceRange ParenRange); + /// SkipUntil - Read tokens until we get to the specified token, then consume /// it (unless DontConsume is true). Because we cannot guarantee that the /// token will ever occur, this skips to the next token, or to some likely diff --git a/lib/Analysis/PathDiagnostic.cpp b/lib/Analysis/PathDiagnostic.cpp index 4fa88ed595..50ff523b81 100644 --- a/lib/Analysis/PathDiagnostic.cpp +++ b/lib/Analysis/PathDiagnostic.cpp @@ -47,6 +47,8 @@ void PathDiagnosticClient::HandleDiagnostic(Diagnostic::Level DiagLevel, 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 bde8a5598b..3d47863e85 100644 --- a/lib/Basic/TokenKinds.cpp +++ b/lib/Basic/TokenKinds.cpp @@ -27,3 +27,66 @@ const char *tok::getTokenName(enum TokenKind Kind) { 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 6a4bf2307f..8ab9b18ecc 100644 --- a/lib/Driver/HTMLDiagnostics.cpp +++ b/lib/Driver/HTMLDiagnostics.cpp @@ -51,7 +51,9 @@ public: 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 = "", + const char *HighlightEnd = ""); void ReportDiag(const PathDiagnostic& D); }; @@ -438,10 +440,33 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID, 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, + "", ""); + } + if (Hint->InsertionLoc.isValid()) { + std::string EscapedCode = html::EscapeText(Hint->CodeToInsert, true); + EscapedCode = "" + EscapedCode + + ""; + 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 @@ void HTMLDiagnostics::HighlightRange(Rewriter& R, FileID BugFileID, SourceLocation E = InstantiationEnd.getFileLocWithOffset(EndColNo - OldEndColNo); - html::HighlightRange(R, InstantiationStart, E, - "", ""); + html::HighlightRange(R, InstantiationStart, E, HighlightStart, HighlightEnd); } diff --git a/lib/Driver/TextDiagnosticPrinter.cpp b/lib/Driver/TextDiagnosticPrinter.cpp index b5226f5131..8a7e40f13e 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 using namespace clang; void TextDiagnosticPrinter:: @@ -104,7 +105,9 @@ void TextDiagnosticPrinter::HighlightRange(const SourceRange &R, 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 @@ void TextDiagnosticPrinter::EmitCaretDiagnostic(SourceLocation Loc, // 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 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 @@ void TextDiagnosticPrinter::HandleDiagnostic(Diagnostic::Level Level, // 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 d0808a5f97..6c6842e0be 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -365,10 +365,19 @@ Parser::ParseRHSOfBinaryExpression(OwningExprResult LHS, unsigned MinPrec) { 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 65705e8d32..0209d7b0ab 100644 --- a/lib/Parse/ParseTemplate.cpp +++ b/lib/Parse/ParseTemplate.cpp @@ -439,8 +439,14 @@ Parser::ParseTemplateIdAfterTemplateName(DeclTy *Template, 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 a3ed027846..a67106c4d9 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -68,6 +68,28 @@ DiagnosticBuilder Parser::Diag(const Token &Tok, unsigned DiagID) { 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 @@ bool Parser::ExpectAndConsume(tok::TokenKind ExpectedTok, unsigned DiagID, 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 0ef998deb1..692af80488 100644 --- a/lib/Rewrite/HTMLRewrite.cpp +++ b/lib/Rewrite/HTMLRewrite.cpp @@ -319,6 +319,9 @@ void html::AddHeaderFooterInternalBuiltinCSS(Rewriter& R, FileID FID, " .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 25f2164ed0..68e0ce8f7d 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -1036,6 +1036,8 @@ public: 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 1f8cb80cd3..eec27cee83 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -110,6 +110,11 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc) { 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 0c60520142..f69f546177 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -1615,12 +1615,10 @@ Sema::ActOnClassTemplateSpecialization(Scope *S, unsigned TagSpec, TagKind TK, // 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(*TemplateParameterLists.get()); if (TemplateParameterLists.size() > 1) { diff --git a/test/Parser/cxx-using-directive.cpp b/test/Parser/cxx-using-directive.cpp index 2ee014e54f..b89436046b 100644 --- a/test/Parser/cxx-using-directive.cpp +++ b/test/Parser/cxx-using-directive.cpp @@ -28,8 +28,9 @@ namespace D { using namespace ! ; // expected-error{{expected namespace name}} using namespace A ; // expected-error{{expected namespace name}} -using namespace ::A // expected-error{{expected namespace name}} - B ; // expected-error{{expected ';' after namespace name}} +using namespace ::A // expected-error{{expected namespace name}} \ + // expected-error{{expected ';' after namespace name}} + B ; void test_nslookup() { int B; diff --git a/test/Parser/recovery.c b/test/Parser/recovery.c index b6ba9e6294..5065b9446a 100644 --- a/test/Parser/recovery.c +++ b/test/Parser/recovery.c @@ -68,3 +68,8 @@ int test6248081() { struct forward; // expected-note{{forward declaration of 'struct forward'}} void x(struct forward* x) {switch(x->a) {}} // expected-error {{incomplete definition of type}} +// PR3410 +void foo() { + int X; + X = 4 // expected-error{{expected ';' after expression}} +} diff --git a/test/SemaTemplate/right-angle-brackets-98.cpp b/test/SemaTemplate/right-angle-brackets-98.cpp index 709e8b05e5..1994e3d965 100644 --- a/test/SemaTemplate/right-angle-brackets-98.cpp +++ b/test/SemaTemplate/right-angle-brackets-98.cpp @@ -9,5 +9,4 @@ X> // expected-error{{a space is required between consecutive right a >> *x3; // expected-error{{a space is required between consecutive right angle brackets (use '> >')}} Y<(1 >> 2)> *y1; -Y<1 >> 2> *y2; -// FIXME: when we get a -Wc++0x mode, warn about the use above +Y<1 >> 2> *y2; // expected-warning{{use of right-shift operator ('>>') in template argument will require parentheses in C++0x}} diff --git a/test/SemaTemplate/temp_arg_nontype.cpp b/test/SemaTemplate/temp_arg_nontype.cpp index d67cc4e409..55815fce1f 100644 --- a/test/SemaTemplate/temp_arg_nontype.cpp +++ b/test/SemaTemplate/temp_arg_nontype.cpp @@ -7,7 +7,7 @@ A *a1; // expected-error{{template argument for non-type template paramet A *a2; // expected-error{{template argument for non-type template parameter must be an expression}} -A<1 >> 2> *a3; +A<1 >> 2> *a3; // expected-warning{{use of right-shift operator ('>>') in template argument will require parentheses in C++0x}} // C++ [temp.arg.nontype]p5: A *a4; // expected-error{{must have an integral or enumeration type}} \ -- 2.40.0