[cfe-commits] r154325 - in /cfe/trunk: include/clang/Parse/Parser.h include/clang/Sema/Initialization.h lib/Parse/ParseTemplate.cpp lib/Parse/ParseTentative.cpp lib/Parse/Parser.cpp lib/Sema/SemaExpr.cpp test/Parser/cxx-template-decl.cpp

David Blaikie dblaikie at gmail.com
Mon Apr 9 09:37:12 PDT 2012


Author: dblaikie
Date: Mon Apr  9 11:37:11 2012
New Revision: 154325

URL: http://llvm.org/viewvc/llvm-project?rev=154325&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/Initialization.h
    cfe/trunk/lib/Parse/ParseTemplate.cpp
    cfe/trunk/lib/Parse/ParseTentative.cpp
    cfe/trunk/lib/Parse/Parser.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Parser/cxx-template-decl.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Mon Apr  9 11:37:11 2012
@@ -725,16 +725,22 @@
   /// 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.

Modified: cfe/trunk/include/clang/Sema/Initialization.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Initialization.h?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Initialization.h (original)
+++ cfe/trunk/include/clang/Sema/Initialization.h Mon Apr  9 11:37:11 2012
@@ -340,7 +340,7 @@
   /// element, sets the element index.
   void setElementIndex(unsigned Index) {
     assert(getKind() == EK_ArrayElement || getKind() == EK_VectorElement ||
-           EK_ComplexElement);
+           getKind() == EK_ComplexElement);
     this->Index = Index;
   }
 

Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTemplate.cpp Mon Apr  9 11:37:11 2012
@@ -309,18 +309,19 @@
   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 @@
       // 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 @@
     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);
     }
   }
   

Modified: cfe/trunk/lib/Parse/ParseTentative.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseTentative.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTentative.cpp Mon Apr  9 11:37:11 2012
@@ -1268,8 +1268,8 @@
     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();
     }
 

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Mon Apr  9 11:37:11 2012
@@ -213,15 +213,14 @@
 ///
 /// 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.

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Apr  9 11:37:11 2012
@@ -7197,7 +7197,7 @@
                                                               &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;

Modified: cfe/trunk/test/Parser/cxx-template-decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-template-decl.cpp?rev=154325&r1=154324&r2=154325&view=diff
==============================================================================
--- cfe/trunk/test/Parser/cxx-template-decl.cpp (original)
+++ cfe/trunk/test/Parser/cxx-template-decl.cpp Mon Apr  9 11:37:11 2012
@@ -9,6 +9,13 @@
 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}}





More information about the cfe-commits mailing list