[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] Fix parsing comma in default arguments.
From:       Olivier Goffart <ogoffart () kde ! org>
Date:       2013-06-13 23:17:22
Message-ID: 1946290.5RIeOljnRS () gargamel
[Download RAW message or body]

On Thursday 13 June 2013 13:52:47 Richard Smith wrote:
> That's not really acceptable, if it can lead to rejecting valid code
> that we accept now (such as your next example).

Ok, the new attached patch fixes that issue.

I'm now quite happy which what (i think) we cover.
It even report good errors when one miss a default argument (on the right 
line)

What is still not working is this:

template <int, int =0> struct p { const static int w = 0; };
struct T {
     int i = p<0,  p<a<b>::w  >::w, p<0>::*j;
     static const int a = 1, b = 2;
};

because a and b are declare after, and the TryAnnotateCXXScopeToken will 
display error as a and b are not defined yet.

But clang did not accept this code before.  And gcc also seems to have a bug 
there.


> > Also a template argument can contains = for example in this case:
> >  struct S {constexpr S(){}; constexpr int operator=(int) const {return
> >  5;}};
> >  template <int> struct A {};
> >  constexpr S s;
> >  A<s=4> a;
> 
> This is ill-formed.

Why?
the operator= is a constexpr

> 
> > But, while GCC accept this code, clang currently reject it.  But that's
> > another bug
> 
> That's a GCC bug. A non-type template argument has to be a
> constant-expression, which is a conditional-expression, and thus
> cannot be an assignment-expression.

but an assignment-expression do not require to semanticly an assignement.

But it should not happen in sensible code

-- 
Olivier

["0001-Fix-parsing-comma-in-default-argument.patch" (0001-Fix-parsing-comma-in-default-argument.patch)]

From cd468c32f97c7a60c826cd6d25b295e2b2fbff63 Mon Sep 17 00:00:00 2001
From: Olivier Goffart <ogoffart@woboq.com>
Date: Sun, 19 May 2013 11:24:54 +0200
Subject: [PATCH] Fix parsing comma in default argument

http://llvm.org/bugs/show_bug.cgi?id=13657
---
 include/clang/Parse/Parser.h              |   1 +
 lib/Parse/ParseCXXInlineMethods.cpp       |  71 +++++++++++++++++--
 lib/Parse/ParseTentative.cpp              |  94 +++++++++++++++++++++++++
 test/Parser/cxx-ambig-init-templ.cpp      | 111 ++++++++++++++++++++++++++++++
 test/Parser/cxx-default-args.cpp          |  17 +++++
 test/Parser/cxx0x-member-initializers.cpp |  10 +++
 6 files changed, 300 insertions(+), 4 deletions(-)
 create mode 100644 test/Parser/cxx-ambig-init-templ.cpp

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 1029a90..4913c3a 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1821,6 +1821,7 @@ private:
   TPResult TryParseParameterDeclarationClause(bool *InvalidAsDeclaration = 0);
   TPResult TryParseFunctionDeclarator();
   TPResult TryParseBracketDeclarator();
+  TPResult TryParseTemplateParameter(bool IsParameterDeclaration);
 
 public:
   TypeResult ParseTypeName(SourceRange *Range = 0,
diff --git a/lib/Parse/ParseCXXInlineMethods.cpp \
b/lib/Parse/ParseCXXInlineMethods.cpp index 5fc4189..c5d75fb 100644
--- a/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/lib/Parse/ParseCXXInlineMethods.cpp
@@ -563,14 +563,58 @@ bool Parser::ConsumeAndStoreUntil(tok::TokenKind T1, \
tok::TokenKind T2,  // We always want this function to consume at least one token if \
the first  // token isn't T and if not at EOF.
   bool isFirstTokenConsumed = true;
+  int TemplateLevel = 0;
   while (1) {
     // If we found one of the tokens, stop and return true.
     if (Tok.is(T1) || Tok.is(T2)) {
-      if (ConsumeFinalToken) {
-        Toks.push_back(Tok);
-        ConsumeAnyToken();
+      bool Done = true;
+      if (Tok.is(tok::comma)) {
+        if (!isFirstTokenConsumed && Toks.back().is(tok::kw_operator)) {
+          // the comma operator does not count when we look for a comma
+          Done = false;
+        } else if (TemplateLevel > 0) {
+          // Is this comma it the separation of template parameters
+          // or is it the separation of function parameters or declarators?
+          TentativeParsingAction TPA(*this);
+          ConsumeToken();
+          // if T2 == r_paren, we are currently parsing the default argument of
+          // a function. Otherwise we are currently parsing an NSDMI.
+          bool IsParameterDeclaraion = T2 == tok::r_paren;
+          TPResult TPR = TryParseTemplateParameter(IsParameterDeclaraion);
+          TPA.Revert();
+
+          if (TPR == TPResult::False() || TPR == TPResult::Error()) {
+            Done = true;
+          } else if (TPR == TPResult::True()) {
+            Done = false;
+          } else {
+              assert(TPR == TPResult::Ambiguous());
+              assert(!IsParameterDeclaraion);
+              // Use another way to disambiguate between
+              // int i = a < b , int e< 0<1 >::* >();
+              // int i = 0<1, e< 0<1 >::*j;
+              TentativeParsingAction TPA(*this);
+              ConsumeToken();
+              // FIXME TryParseDeclaration will throw error on undefined
+              // identifiers that would be defined later in the class.
+              if (TryParseDeclarator(false) != TPResult::Ambiguous()) {
+                Done = false;
+              } else {
+                //after the declarator must come one of these tokens:
+                Done = Tok.is(tok::comma) || Tok.is(tok::equal) || Tok.is(tok::semi) \
|| Tok.is(tok::colon) || Tok.is(tok::l_brace) ; +              }
+
+              TPA.Revert();
+          }
+        }
+      }
+      if (Done) {
+        if (ConsumeFinalToken) {
+          Toks.push_back(Tok);
+          ConsumeAnyToken();
+        }
+        return true;
       }
-      return true;
     }
 
     switch (Tok.getKind()) {
@@ -597,6 +641,25 @@ bool Parser::ConsumeAndStoreUntil(tok::TokenKind T1, \
tok::TokenKind T2,  ConsumeAndStoreUntil(tok::r_brace, Toks, /*StopAtSemi=*/false);
       break;
 
+    case tok::less:
+      if (isFirstTokenConsumed || !Toks.back().is(tok::kw_operator))
+        TemplateLevel++;
+      Toks.push_back(Tok);
+      ConsumeToken();
+      break;
+    case tok::greater:
+      if (TemplateLevel && (isFirstTokenConsumed || \
!Toks.back().is(tok::kw_operator))) +        TemplateLevel--;
+      Toks.push_back(Tok);
+      ConsumeToken();
+      break;
+    case tok::greatergreater:
+      if (TemplateLevel && (isFirstTokenConsumed || \
!Toks.back().is(tok::kw_operator))) +        TemplateLevel-=2;
+      Toks.push_back(Tok);
+      ConsumeToken();
+      break;
+
     // Okay, we found a ']' or '}' or ')', which we think should be balanced.
     // Since the user wasn't looking for this token (if they were, it would
     // already be handled), this isn't balanced.  If there is a LHS token at a
diff --git a/lib/Parse/ParseTentative.cpp b/lib/Parse/ParseTentative.cpp
index dff3b64..22af8bd 100644
--- a/lib/Parse/ParseTentative.cpp
+++ b/lib/Parse/ParseTentative.cpp
@@ -1530,6 +1530,100 @@ Parser::TryParseParameterDeclarationClause(bool \
*InvalidAsDeclaration) {  return TPResult::Ambiguous();
 }
 
+/// try to disambiguate between a comma between template arguments and a comma
+/// between function parameters.
+/// The comma was already parsed.
+///
+/// IsParameterDeclaration indicates if we are in a default parameter
+/// declaration, or in a NSDMI context
+///
+Parser::TPResult Parser::TryParseTemplateParameter(bool IsParameterDeclaration) {
+  // Now try to find the '=' sign of the next parameter, or the '>' of the end
+  // of the template parameter list, whatever comes first tells us what it is.
+  tok::TokenKind TokArray[] = {tok::equal, tok::less, tok::comma,
+                               tok::greater, tok::greatergreater,
+                               tok::kw_operator, tok::coloncolon };
+
+  int AngleBracketDepth = 0;
+  bool SeenGreater = false;
+  bool SeenPointerToMember = false;
+
+  while (SkipUntil(TokArray, /*StopAtSemi*/ true, /*DontConsume*/ true)) {
+
+    switch(Tok.getKind()) {
+    case tok::equal:
+      // The start of the next default argument: it was not a template
+      return TPResult::False();
+
+    case tok::less:
+      AngleBracketDepth++;
+      break;
+
+    case tok::greater:
+      SeenGreater = true;
+      AngleBracketDepth--;
+      break;
+
+    case tok::greatergreater:
+      if(getLangOpts().CPlusPlus11) {
+        SeenGreater = true;
+        AngleBracketDepth -= 2;
+      }
+      break;
+
+    case tok::comma:
+      // When parsing parameter declarationa, another comma
+      // can only appears in template parameter
+      if (!AngleBracketDepth && IsParameterDeclaration)
+        return TPResult::True();
+      break;
+
+    case tok::kw_operator:
+      //Ignore one token.
+      ConsumeToken();
+      if (Tok.is(tok::eof) || Tok.is(tok::semi))
+        return TPResult::Error();
+      break;
+
+    case tok::coloncolon:
+      if (IsParameterDeclaration || !SeenGreater)
+        break;
+      ConsumeToken();
+      // Do we have '::*' ?
+      if (Tok.is(tok::star))
+        SeenPointerToMember = true;
+      continue; // We already consumed.
+
+    default:
+      assert(!"unexpected token");
+    }
+
+    // There cannot be '>' if we are not in a template argument.
+    if (AngleBracketDepth < 0)
+     return TPResult::True();
+
+    ConsumeToken();
+  }
+
+  if (IsParameterDeclaration) {
+    // if neither '>' nor '=' was seen, it is an error.
+    return SeenGreater ? TPResult::True() : TPResult::Error();
+  }
+
+  // if '>' was not seen at all, it can't be a template argument
+  if (!SeenGreater)
+    return TPResult::False();
+
+  // '>' or '<' can only apprear in declarators in pointer to member
+  if (!SeenPointerToMember)
+    return TPResult::True();
+
+  // Could still be ambiguous:
+  // int i = a < b , int e< 0<1 >::* >();
+  // int i = 0<1, e< 0<1 >::*j;
+  return TPResult::Ambiguous();
+}
+
 /// TryParseFunctionDeclarator - We parsed a '(' and we want to try to continue
 /// parsing as a function declarator.
 /// If TryParseFunctionDeclarator fully parsed the function declarator, it will
diff --git a/test/Parser/cxx-ambig-init-templ.cpp \
b/test/Parser/cxx-ambig-init-templ.cpp new file mode 100644
index 0000000..6866ccd
--- /dev/null
+++ b/test/Parser/cxx-ambig-init-templ.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+template<int> struct c { c(int) = delete; typedef void val; };
+
+int val;
+struct S {
+
+  int k1 = a < b < c, d > ::val, e1;
+  int k2 = a < b, c < d > ::val, e2;
+  int k3 = b < a < c, d > ::val, e3;
+  int k4 = b < c, x, y = d > ::val, e4;
+  int k5 = T1 < b, &operator=(int);
+  int k6 = T2 < b, &S::operator= >::val;
+  int k7 = T1 < b, &operator>(int);
+  int k8 = T2 < b, &S::operator> >::val;
+  int k9 = T3 < a < b, c >> (d), e5 = 1 > (e4);
+  int k10 = 0 < 1, c<3>::*ptr;
+  int k11 = e < 0, int a<b<c>::* >(), e11;
+
+ template<typename> struct T3 { T3(int); operator int(); };
+
+  void f(
+    int k1 = a < b < c, d > ::val,
+    int k3 = b < a < c, d > ::val,
+    int k4 = b < c, int x = 0 > ::val,
+    int k5 = e < b, T3 < int > >(),
+    int k6 = c < b, T3 < int > x2 = 0,
+    int k2 = a < b, c < d > ::val,
+    int k7 = a < b, c < d > ()
+  );
+
+  int f2(int k2 = a < b, c < d > ::val);
+
+  void f3(
+    int k1 = a<1,2>::val,
+    int missing_default  // expected-error {{missing default argument on parameter}}
+  );
+
+  void f4(
+    int k1 = b < c,
+    int missing_default  // expected-error {{missing default argument on parameter}}
+  );
+
+  static const int b = 0, c = 1, d = 2;
+  template<int, int=0> struct a { static const int val = 0; operator int();};
+  template<int, typename> struct e { operator int(); };
+
+  static const int T1 = 4;
+  template<int, int &(S::*)(int)> struct T2 { static const int val = 0; };
+
+
+  int mp1 = 0 < 1,
+      a<b<c,b<c>::*mp2,
+      mp3 = 0 > a<b<c>::val,
+      a<b<c,b<c>::*mp4 = 0,
+      a<b<c,b<c>::*mp5 {0},
+      a<b<c,b<c>::*mp6;
+
+  int np1 = e<0, int a<b<c,b<c>::*>();
+
+};
+
+namespace CWG325 {
+
+  template <int A, typename B> struct T { static int i; operator int(); };
+  class C {
+    int Foo (int i = T<1, int>::i);
+  };
+
+  class D {
+    int Foo (int i = T<1, int>::i);
+    template <int A, typename B> struct T {static int i;};
+  };
+
+  const int a = 0;
+  typedef int b;
+  T<a,b> c;
+  struct E {
+    int n = T<a,b>(c);
+  };
+
+}
+
+namespace Operators {
+
+  struct Y {};
+  constexpr int operator,(const Y&, const Y&) { return 8; }
+  constexpr int operator>(const Y&, const Y&) { return 8; }
+  constexpr int operator<(const Y&, const Y&) { return 8; }
+
+  int x (const Y&, const Y&);
+
+  struct X {
+
+
+    typedef int (*Fn) (const Y&, const Y&);
+
+    Fn com = operator, , les = operator<, gre = operator >;
+    void f1(Fn a = operator, , Fn b = operator< , Fn c = operator>);
+
+    int k1 = T1<0, operator< , operator> , operator< >::val, l1;
+    int k2 = T1<0, operator> , operator, , operator, >::val, l2;
+    int k3 = T2<0, operator,(Y{}, Y{}),  operator<(Y{}, Y{})>::val, l3;
+    int k4 = T2<0, operator>(Y{}, Y{}),  operator,(Y{}, Y{})>::val, l4;
+
+    template <int, Fn, Fn, Fn> struct T1 { enum { val }; };
+    template <int, int, int> struct T2 { enum { val }; };
+  };
+
+}
+
diff --git a/test/Parser/cxx-default-args.cpp b/test/Parser/cxx-default-args.cpp
index 7fe8474..19b50cb 100644
--- a/test/Parser/cxx-default-args.cpp
+++ b/test/Parser/cxx-default-args.cpp
@@ -14,3 +14,20 @@ typedef struct Inst {
 struct X {
   void f(int x = 1:); // expected-error {{unexpected end of default argument \
expression}}  };
+
+// PR13657
+struct T {
+  template <typename A, typename B> struct T1 { enum {V};};
+  template <int A, int B> struct T2 { enum {V}; };
+  template <int, int> static int func(int);
+
+
+  void f1(T1<int, int> = T1<int, int>());
+  void f2(T1<int, double> = T1<int, double>(), T2<0, 5> = T2<0, 5>());
+  void f3(int a = T2<0, (T1<int, int>::V > 10) ? 5 : 6>::V, bool b = 4<5 );
+  void f4(bool a = 1 < 0, bool b = 2 > 0 );
+  void f5(bool a = 1 > T2<0, 0>::V, bool b = T1<int,int>::V < 3, int c = 0);
+  void f6(bool a = T2<0,3>::V < 4, bool b = 4 > T2<0,3>::V);
+  void f7(bool a = T1<int, bool>::V < 3);
+  void f8(int = func<0,1<2>(0), int = 1<0, T1<int,int>(int) = 0);
+};
\ No newline at end of file
diff --git a/test/Parser/cxx0x-member-initializers.cpp \
b/test/Parser/cxx0x-member-initializers.cpp index a324f97..43e99b1 100644
--- a/test/Parser/cxx0x-member-initializers.cpp
+++ b/test/Parser/cxx0x-member-initializers.cpp
@@ -27,3 +27,13 @@ struct V1 {
   int a, b;
   V1() : a(), b{} {}
 };
+
+template <typename, typename> struct T1 { enum {V};};
+template <int, int> struct T2 { enum {V};};
+struct A {
+  T1<int, int> a1 = T1<int, int>(), *a2 = new T1<int,int>;
+  T2<0,0> b1 = T2<0,0>(), b2 = T2<0,0>(), b3;
+  bool c1 = 1 < 2, c2 = 2 < 1, c3 = false;
+  bool d1 = T1<int, T1<int, int>>::V < 3, d2;
+  T1<int, int()> e = T1<int, int()>();
+};
-- 
1.7.12.1



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic