From eb52f86a62db523e3c993686b3ed92c55d59d53c Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 9 Apr 2012 16:37:11 +0000 Subject: [PATCH] Fix bugs found by -Wconstant-conversion improvements currently under review. Specifically, using a an integer outside [0, 1] as a boolean constant seems to be an easy mistake to make with things like "x == a || b" where the author intended "x == a || x == b". The bug caused by calling SkipUntil with three token kinds was also identified by a VC diagnostic & reported by Francois Pichet as review feedback for my commit r154163. I've included test cases to verify the error recovery that was broken/poorly implemented due to this bug. The other fix (lib/Sema/SemaExpr.cpp) seems like that code was never actually reached in any of Clang's tests & is related to Objective C features I'm not familiar with, so I've not been able to construct a test case for it. Perhaps someone else can. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@154325 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Parse/Parser.h | 14 +++++++++---- include/clang/Sema/Initialization.h | 2 +- lib/Parse/ParseTemplate.cpp | 32 +++++++++++++---------------- lib/Parse/ParseTentative.cpp | 4 ++-- lib/Parse/Parser.cpp | 7 +++---- lib/Sema/SemaExpr.cpp | 2 +- test/Parser/cxx-template-decl.cpp | 7 +++++++ 7 files changed, 38 insertions(+), 30 deletions(-) diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 892d5fa501..0721417f7b 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -725,16 +725,22 @@ private: /// returns false. bool SkipUntil(tok::TokenKind T, bool StopAtSemi = true, bool DontConsume = false, bool StopAtCodeCompletion = false) { - return SkipUntil(&T, 1, StopAtSemi, DontConsume, StopAtCodeCompletion); + return SkipUntil(llvm::makeArrayRef(T), StopAtSemi, DontConsume, + StopAtCodeCompletion); } bool SkipUntil(tok::TokenKind T1, tok::TokenKind T2, bool StopAtSemi = true, bool DontConsume = false, bool StopAtCodeCompletion = false) { tok::TokenKind TokArray[] = {T1, T2}; - return SkipUntil(TokArray, 2, StopAtSemi, DontConsume,StopAtCodeCompletion); + return SkipUntil(TokArray, StopAtSemi, DontConsume,StopAtCodeCompletion); } - bool SkipUntil(const tok::TokenKind *Toks, unsigned NumToks, + bool SkipUntil(tok::TokenKind T1, tok::TokenKind T2, tok::TokenKind T3, bool StopAtSemi = true, bool DontConsume = false, - bool StopAtCodeCompletion = false); + bool StopAtCodeCompletion = false) { + tok::TokenKind TokArray[] = {T1, T2, T3}; + return SkipUntil(TokArray, StopAtSemi, DontConsume,StopAtCodeCompletion); + } + bool SkipUntil(ArrayRef Toks, bool StopAtSemi = true, + bool DontConsume = false, bool StopAtCodeCompletion = false); //===--------------------------------------------------------------------===// // Lexing and parsing of C++ inline methods. diff --git a/include/clang/Sema/Initialization.h b/include/clang/Sema/Initialization.h index 6d92df6322..23cc4455db 100644 --- a/include/clang/Sema/Initialization.h +++ b/include/clang/Sema/Initialization.h @@ -340,7 +340,7 @@ public: /// element, sets the element index. void setElementIndex(unsigned Index) { assert(getKind() == EK_ArrayElement || getKind() == EK_VectorElement || - EK_ComplexElement); + getKind() == EK_ComplexElement); this->Index = Index; } diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp index 9d5c62567e..61cd9f2119 100644 --- a/lib/Parse/ParseTemplate.cpp +++ b/lib/Parse/ParseTemplate.cpp @@ -309,18 +309,19 @@ bool Parser::ParseTemplateParameters(unsigned Depth, LAngleLoc = ConsumeToken(); // Try to parse the template parameter list. - if (Tok.is(tok::greater)) + bool Failed = false; + if (!Tok.is(tok::greater) && !Tok.is(tok::greatergreater)) + Failed = ParseTemplateParameterList(Depth, TemplateParams); + + if (Tok.is(tok::greatergreater)) { + Tok.setKind(tok::greater); + RAngleLoc = Tok.getLocation(); + Tok.setLocation(Tok.getLocation().getLocWithOffset(1)); + } else if (Tok.is(tok::greater)) RAngleLoc = ConsumeToken(); - else if (ParseTemplateParameterList(Depth, TemplateParams)) { - if (Tok.is(tok::greatergreater)) { - Tok.setKind(tok::greater); - Tok.setLocation(Tok.getLocation().getLocWithOffset(1)); - } else if (Tok.is(tok::greater)) - RAngleLoc = ConsumeToken(); - else { - Diag(Tok.getLocation(), diag::err_expected_greater); - return true; - } + else if (Failed) { + Diag(Tok.getLocation(), diag::err_expected_greater); + return true; } return false; } @@ -356,10 +357,8 @@ Parser::ParseTemplateParameterList(unsigned Depth, // Somebody probably forgot to close the template. Skip ahead and // try to get out of the expression. This error is currently // subsumed by whatever goes on in ParseTemplateParameter. - // TODO: This could match >>, and it would be nice to avoid those - // silly errors with template >. Diag(Tok.getLocation(), diag::err_expected_comma_greater); - SkipUntil(tok::greater, true, true); + SkipUntil(tok::comma, tok::greater, tok::greatergreater, true, true); return false; } } @@ -607,10 +606,7 @@ Parser::ParseTemplateTemplateParameter(unsigned Depth, unsigned Position) { if (DefaultArg.isInvalid()) { Diag(Tok.getLocation(), diag::err_default_template_template_parameter_not_template); - static const tok::TokenKind EndToks[] = { - tok::comma, tok::greater, tok::greatergreater - }; - SkipUntil(EndToks, 3, true, true); + SkipUntil(tok::comma, tok::greater, tok::greatergreater, true, true); } } diff --git a/lib/Parse/ParseTentative.cpp b/lib/Parse/ParseTentative.cpp index 73501e58ca..b650169b25 100644 --- a/lib/Parse/ParseTentative.cpp +++ b/lib/Parse/ParseTentative.cpp @@ -1268,8 +1268,8 @@ Parser::TPResult Parser::TryParseParameterDeclarationClause() { if (Tok.is(tok::equal)) { // '=' assignment-expression // Parse through assignment-expression. - tok::TokenKind StopToks[2] ={ tok::comma, tok::r_paren }; - if (!SkipUntil(StopToks, 2, true/*StopAtSemi*/, true/*DontConsume*/)) + if (!SkipUntil(tok::comma, tok::r_paren, true/*StopAtSemi*/, + true/*DontConsume*/)) return TPResult::Error(); } diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 2c6dff35b2..bc515cab5d 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -213,15 +213,14 @@ bool Parser::ExpectAndConsumeSemi(unsigned DiagID) { /// /// If SkipUntil finds the specified token, it returns true, otherwise it /// returns false. -bool Parser::SkipUntil(const tok::TokenKind *Toks, unsigned NumToks, - bool StopAtSemi, bool DontConsume, - bool StopAtCodeCompletion) { +bool Parser::SkipUntil(ArrayRef Toks, bool StopAtSemi, + bool DontConsume, bool StopAtCodeCompletion) { // We always want this function to skip at least one token if the first token // isn't T and if not at EOF. bool isFirstTokenSkipped = true; while (1) { // If we found one of the tokens, stop and return true. - for (unsigned i = 0; i != NumToks; ++i) { + for (unsigned i = 0, NumToks = Toks.size(); i != NumToks; ++i) { if (Tok.is(Toks[i])) { if (DontConsume) { // Noop, don't consume the token. diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index fea0fe1e62..1d8d24882e 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -7197,7 +7197,7 @@ static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) { &Loc); if (IsLV == Expr::MLV_Valid && IsReadonlyProperty(E, S)) IsLV = Expr::MLV_ReadonlyProperty; - else if (Expr::MLV_ConstQualified && IsConstProperty(E, S)) + else if (IsLV == Expr::MLV_ConstQualified && IsConstProperty(E, S)) IsLV = Expr::MLV_Valid; else if (IsLV == Expr::MLV_ClassTemporary && IsReadonlyMessage(E, S)) IsLV = Expr::MLV_InvalidMessageExpression; diff --git a/test/Parser/cxx-template-decl.cpp b/test/Parser/cxx-template-decl.cpp index be1f81e9d1..7e931a31fa 100644 --- a/test/Parser/cxx-template-decl.cpp +++ b/test/Parser/cxx-template-decl.cpp @@ -9,6 +9,13 @@ export template class x0; // expected-warning {{exported templates are template < ; // expected-error {{expected template parameter}} \ // expected-error{{expected ',' or '>' in template-parameter-list}} \ // expected-warning {{declaration does not declare anything}} +template struct x1; // expected-error {{expected ',' or '>' in template-parameter-list}} + +// verifies that we only walk to the ',' & still produce errors on the rest of the template parameters +template struct x2; // expected-error {{expected ',' or '>' in template-parameter-list}} \ + expected-error {{expected unqualified-id}} +template> struct x3; // expected-error {{expected ',' or '>' in template-parameter-list}} \ + expected-error {{template template parameter requires 'class' after the parameter list}} template