[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 19:47:55
Message-ID: 3935367.akZ5kgb8YR () gargamel
[Download RAW message or body]

On Wednesday 12 June 2013 14:39:04 Richard Smith wrote:
> I've had a look at this before. Here are some testcases for you:
> 
> Index: test/Parser/cxx-ambig-init-templ.cpp
> ===================================================================
> --- test/Parser/cxx-ambig-init-templ.cpp        (revision 0)
> +++ test/Parser/cxx-ambig-init-templ.cpp        (revision 0)
> @@ -0,0 +1,57 @@
> +// 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, &S::operator=(int); // expected-warning
> {{qualification}} 

that is an error instead of a warning for me, so i removed the extra 
qualification

> +  int k6 = T2 < b, &S::operator= >::val;
> +  int k7 = T1 < b, &S::operator>(int); // expected-warning
> {{qualification}} 

Same here.

> +  int k8 = T2 < b, &S::operator> >::val;
> +  int k9 = T3 < a < b, c >> (d), e5 = 1 > (e4);
> +
> +  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 = a < b, T3 < int > >(),

T3<int>  is not int,  and a is not an int

> +    int k6 = c < b, T3 < int > x,

'x is a duplicate argument, missing a default value, and T3 must be declared 
before

> +
> +    int k2 = a < b, c < d > ::val, // 'a' must be a template
> (otherwise we're missing a default arg)
> +    int k7 = a < b, c < d > (n) // 'a' must be a template (otherwise
> we're missing a default arg)

there is no 'n' and 'a' don't  convert to int

> +  );
> +
> +  template<int, int=0> struct a { static const int val = 0; };
> +  static const int b = 0, c = 1, d = 2;
> +
> +  static const int T1 = 4;
> +  template<int, int &(S::*)(int)> struct T2 { static const int val = 0; };
> +
> +  template<typename> struct T3 { T3(int); operator int(); };
> +};
> +
> +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);
> +  };
> +
> +}
> 
> I don't see any handling of "operator<", "operator>", "operator,",
> "operator=" in your patch, so I assume it doesn't handle those cases
> properly. (That should be straightforward to fix.)

Right, I have forgotten that.

> Essentially, there are two different tricks we need in order to fully
> disambiguate these cases with no name lookup:
> 
> 1) For an in-class function declaration, if we are in a default
> argument, every parameter after the current parameter must also have a
> default argument, and a template argument cannot contain a top-level
> '='. So if we see a '=', we know we're back to a parameter, and can
> back up to the comma at the start of that parameter.

This was basically what i implemented before
 
> 2) For an in-class default initializer for a data member, any '=' must
> be part of a new data member, and otherwise any '<' or '>' must be
> part of the same initializer (because a declarator cannot contain a
> '<' or '>').

Yes, the previous patch did not really work well with NSDMI.

Too be honnest I was mostly focussed on getting the default argument working 
since it was the problem i had on the project I was working with.

Now it is implemented using a similar way.

I can still think about some broken cases, but I think they are corner case 
and they do not deserve to be fixed:
 int i = 0 < 1,   vector<string>::*j;


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;

But, while GCC accept this code, clang currently reject it.  But that's 
another bug

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

From 1f5fb3c53cfdb2f9a02d492b4217aaad017d7d51 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       | 56 +++++++++++++++++--
 lib/Parse/ParseTentative.cpp              | 64 +++++++++++++++++++++
 test/Parser/cxx-ambig-init-templ.cpp      | 93 +++++++++++++++++++++++++++++++
 test/Parser/cxx-default-args.cpp          | 17 ++++++
 test/Parser/cxx0x-member-initializers.cpp | 10 ++++
 6 files changed, 237 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..07060ab 100644
--- a/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/lib/Parse/ParseCXXInlineMethods.cpp
@@ -563,14 +563,43 @@ 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);
+
+          if (TPR == TPResult::False()) {
+            Done = true;
+          } else if (TPR == TPResult::True()) {
+            Done = false;
+          } else {
+            Done = !IsParameterDeclaraion;
+          }
+
+          TPA.Revert();
+        }
+      }
+      if (Done) {
+        if (ConsumeFinalToken) {
+          Toks.push_back(Tok);
+          ConsumeAnyToken();
+        }
+        return true;
       }
-      return true;
     }
 
     switch (Tok.getKind()) {
@@ -597,6 +626,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..57dc4d8 100644
--- a/lib/Parse/ParseTentative.cpp
+++ b/lib/Parse/ParseTentative.cpp
@@ -1530,6 +1530,70 @@ 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 };
+
+  int AngleBracketDepth = 0;
+
+  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:
+      AngleBracketDepth--;
+      break;
+
+    case tok::greatergreater:
+      if(getLangOpts().CPlusPlus11)
+        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;
+
+    default:
+      assert(!"unexpected token");
+    }
+
+    // There cannot be '>' if we are not in a template argument.
+    if (AngleBracketDepth < 0 || (!IsParameterDeclaration && AngleBracketDepth != 0))
+     return TPResult::True();
+
+    ConsumeToken();
+  }
+
+  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..4a93a40
--- /dev/null
+++ b/test/Parser/cxx-ambig-init-templ.cpp
@@ -0,0 +1,93 @@
+// 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);
+
+ 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}}
+  );
+
+  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; };
+};
+
+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