[PATCH] Fix parsing comma in default arguments.

[Richard Smith]

On Thu, Jun 13, 2013 at 12:47 PM, Olivier Goffart <ogoffart at kde.org> wrote:
> 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

Yes, this test case was written a while ago (and is incomplete and not
entirely correct). You can demote this to a warning with
-fms-extensions.

>> +  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

T3 has a conversion to int (which should be marked constexpr but
isn't), and 'a' should also have one.

>> +    int k6 = c < b, T3 < int > x,
>
> 'x is a duplicate argument, missing a default value, and T3 must be declared
> before

Sure.

>> +
>> +    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:

That's not really acceptable, if it can lead to rejecting valid code
that we accept now (such as your next example).

>  int i = 0 < 1,   vector<string>::*j;

Yuck. But that is still unambiguous.

> 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.

> 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.