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

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 892d5fa501bc69ebe69037b41ec4a1681165b971..0721417f7ba4e13ea757261c531c0a43c1330345 100644 (file)
--- 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<tok::TokenKind> 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 6d92df63228eb793b3e523356b7c24e0ee691d7b..23cc4455dbc21fb82cbc4e2381ef293276daec43 100644 (file)
--- 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 9d5c62567e4938471e8798b10d84bc47fe8b6dbf..61cd9f2119a6565a3ee8cd3116f8d355651b9c18 100644 (file)
--- 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 <vec<T>>.
       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 73501e58ca30befd1a747566037598a1504388de..b650169b2592fb8655d5d3dd933d9b9ffb7461ef 100644 (file)
--- 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 2c6dff35b272346f46be1347e2cd5db5e33e3359..bc515cab5d537547bf2c04552382de50f2430c83 100644 (file)
--- 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<tok::TokenKind> 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 fea0fe1e62b3c1016b6dbdafde247874311820df..1d8d24882e5ceaf517ce6eaebbcbc3cfa0e0f4f3 100644 (file)
--- 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 be1f81e9d15a516a4fbef731c3e008ba20acacfd..7e931a31fa2d5170a8f7a086e3709f2cd8db1243 100644 (file)
--- a/test/Parser/cxx-template-decl.cpp
+++ b/test/Parser/cxx-template-decl.cpp
@@ -9,6 +9,13 @@ export template<class T> 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 <int +> 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 <int +, T> struct x2; // expected-error {{expected ',' or '>' in template-parameter-list}} \
+                                expected-error {{expected unqualified-id}}
+template<template<int+>> struct x3; // expected-error {{expected ',' or '>' in template-parameter-list}} \
+                                         expected-error {{template template parameter requires 'class' after the parameter list}}
 template <template X> struct Err1; // expected-error {{expected '<' after 'template'}} \
 // expected-error{{extraneous}}
 template <template <typename> > struct Err2;       // expected-error {{template template parameter requires 'class' after the parameter list}}