Re: [PATCH v4] c++: Handle auto(x) in parameter-declaration-clause [PR103401]

Jason Merrill via Gcc-patches Thu, 09 Dec 2021 07:56:20 -0800

On 12/8/21 18:23, Marek Polacek wrote:

On Wed, Dec 08, 2021 at 03:09:00PM -0500, Jason Merrill wrote:

On 12/8/21 13:32, Marek Polacek wrote:

On Wed, Dec 08, 2021 at 09:15:05AM -0500, Jason Merrill wrote:

On 12/7/21 19:25, Marek Polacek wrote:

On Mon, Dec 06, 2021 at 04:44:06PM -0500, Jason Merrill wrote:

Please also make this change to cp_parser_sizeof_operand, and add tests
involving sizeof/alignof in array bounds.  OK with that change.


Turns out we reject sizeof(auto(4)) because cp_parser_type_id_1 errors
"invalid use of auto".  So I've added a hack to make it work; auto(x)
is *not* a type-id, so reject that parse and let it be parsed as an
expression.

FWIW, I don't think we need to clear auto_is_implicit_function_template_parm_p
in cp_parser_sizeof_operand for parameters like int[sizeof(auto(10))] because
the auto is in a declarator and auto_is_... will have been cleared already in
cp_parser_parameter_declaration before parsing the declarator.  But I've added
it anyway, maybe there are other cases where it matters.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
In C++23, auto(x) is valid, so decltype(auto(x)) should also be valid,
so

     void f(decltype(auto(0)));

should be just as

     void f(int);

but currently, everytime we see 'auto' in a parameter-declaration-clause,
we try to synthesize_implicit_template_parm for it, creating a new template
parameter list.  The code above actually has us calling s_i_t_p twice;
once from cp_parser_decltype_expr -> cp_parser_postfix_expression which
fails and then again from cp_parser_decltype_expr -> cp_parser_expression.
So it looks like we have f<auto, auto> and we accept ill-formed code.

This shows that we need to be more careful about synthesizing the
implicit template parameter.  [dcl.spec.auto.general] says that "A
placeholder-type-specifier of the form type-constraintopt auto can be
used as a decl-specifier of the decl-specifier-seq of a
parameter-declaration of a function declaration or lambda-expression..."
so this patch turns off auto_is_... after we've parsed the decl-specifier-seq.

That doesn't quite cut it yet though, because we also need to handle an
auto nested in the decl-specifier:

     void f(decltype(new auto{0}));

therefore the cp_parser_decltype change.

To accept "sizeof(auto{10})", the cp_parser_type_id_1 hunk rejects the
current parse if it sees an auto followed by a ( or {.


The problem here doesn't seem specific to the ( or {, but that we're giving
a hard error in tentative parsing context; I think we want to guard that
error with cp_parser_simulate_error like we do a few lines earlier for class
template placeholders.


I agree that that's generally the approach that makes sense, but in this
case it regresses our diagnostic :(.  For example,

    int i = *(auto *) 0;

would give

q.C:1:11: error: expected primary-expression before ‘auto’
      1 | int i = *(auto *) 0;
        |           ^~~~
q.C:1:11: error: expected ‘)’ before ‘auto’
      1 | int i = *(auto *) 0;
        |          ~^~~~
        |           )

instead of the current

q.C:1:11: error: invalid use of ‘auto’
      1 | int i = *(auto *) 0;
        |           ^~~~

We just reject the parse in cp_parser_type_id_1 and then give an error in
cp_parser_primary_expression:

    cp_parser_error (parser, "expected primary-expression");

I suppose I could add 'case RID_AUTO' to cp_parser_primary_expression and
issue an error there, but that doesn't understand decltype(auto) etc, and
still issues multiple error messages.


Or, maybe it would be OK to actually go with the cp_parser_simulate_error
approach and accept that about 5 tests produce somewhat worse diagnostic.

What's your preference?


Hmm.

auto( could be the beginning of e.g. auto(*)(), which is also a type-id, and
might trip your assert instead of giving an error.


Ah, yes.

So I think the latter is the way to go.


Patch attached.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

I wonder about some time establishing a pattern in the parser that if a
tentative parse results in error_mark_node without simulating an error, we
repeat the same parse again to get the desired semantic error.  But that's a
big project, not something to address this bug.


Interesting.  How would we handle e.g. the case when sizeof gets something
that isn't a valid type-id, so we reject the parse, try to parse it as an
expression, but that fails too -- parse again as a type-id, this time with
errors?  It will probably be hard to say if it's more of a type-id or more
of an expression.  I hope it would improve our parsing of template arguments
where we sometimes just print "parse error".  :]

I was thinking of situations where the argument is a syntactically validtype-id that violates some constraint, as in this case, so we wouldnever try to parse as an expression.

-- >8 --
In C++23, auto(x) is valid, so decltype(auto(x)) should also be valid,
so

   void f(decltype(auto(0)));

should be just as

   void f(int);

but currently, everytime we see 'auto' in a parameter-declaration-clause,
we try to synthesize_implicit_template_parm for it, creating a new template
parameter list.  The code above actually has us calling s_i_t_p twice;
once from cp_parser_decltype_expr -> cp_parser_postfix_expression which
fails and then again from cp_parser_decltype_expr -> cp_parser_expression.
So it looks like we have f<auto, auto> and we accept ill-formed code.

This shows that we need to be more careful about synthesizing the
implicit template parameter.  [dcl.spec.auto.general] says that "A
placeholder-type-specifier of the form type-constraintopt auto can be
used as a decl-specifier of the decl-specifier-seq of a
parameter-declaration of a function declaration or lambda-expression..."
so this patch turns off auto_is_... after we've parsed the decl-specifier-seq.

That doesn't quite cut it yet though, because we also need to handle an
auto nested in the decl-specifier:

   void f(decltype(new auto{0}));

therefore the cp_parser_decltype change.

To accept "sizeof(auto{10})", the cp_parser_type_id_1 hunk only gives a
hard error when we're not parsing tentatively.

The cp_parser_parameter_declaration hunk broke lambda-generic-85713-2.C but
I think the error we issue with this patch is in fact correct, and clang++
agrees.

The r11-1913 change is OK: we need to make sure that we see '(auto)' after
decltype to go ahead with 'decltype(auto)'.

        PR c++/103401

gcc/cp/ChangeLog:

        * parser.c (cp_parser_decltype): Clear
        auto_is_implicit_function_template_parm_p.
        (cp_parser_type_id_1): Give errors only when !cp_parser_simulate_error.
        (cp_parser_parameter_declaration): Clear
        auto_is_implicit_function_template_parm_p after parsing the
        decl-specifier-seq.
        (cp_parser_sizeof_operand): Clear
        auto_is_implicit_function_template_parm_p.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp1y/lambda-generic-85713-2.C: Add dg-error.
        * g++.dg/cpp1y/pr60054.C: Adjust dg-error.
        * g++.dg/cpp1y/pr60332.C: Likewise.
        * g++.dg/cpp2a/concepts-pr84979-2.C: Likewise.
        * g++.dg/cpp2a/concepts-pr84979-3.C: Likewise.
        * g++.dg/cpp2a/concepts-pr84979.C: Likewise.
        * g++.dg/cpp23/auto-fncast7.C: New test.
        * g++.dg/cpp23/auto-fncast8.C: New test.
        * g++.dg/cpp23/auto-fncast9.C: New test.
---
  gcc/cp/parser.c                               | 38 +++++++++++++----
  .../g++.dg/cpp1y/lambda-generic-85713-2.C     |  2 +-
  gcc/testsuite/g++.dg/cpp1y/pr60054.C          |  4 +-
  gcc/testsuite/g++.dg/cpp1y/pr60332.C          |  3 +-
  gcc/testsuite/g++.dg/cpp23/auto-fncast7.C     |  9 ++++
  gcc/testsuite/g++.dg/cpp23/auto-fncast8.C     | 42 +++++++++++++++++++
  gcc/testsuite/g++.dg/cpp23/auto-fncast9.C     | 17 ++++++++
  .../g++.dg/cpp2a/concepts-pr84979-2.C         | 12 +++---
  .../g++.dg/cpp2a/concepts-pr84979-3.C         | 12 +++---
  gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C |  2 +-
  10 files changed, 116 insertions(+), 25 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp23/auto-fncast7.C
  create mode 100644 gcc/testsuite/g++.dg/cpp23/auto-fncast8.C
  create mode 100644 gcc/testsuite/g++.dg/cpp23/auto-fncast9.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6f273bfe21f..de464afdb54 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -16432,6 +16432,16 @@ cp_parser_decltype (cp_parser *parser)
        = parser->greater_than_is_operator_p;
        parser->greater_than_is_operator_p = true;

+ /* Don't synthesize an implicit template type parameter here. This

+        could happen with C++23 code like
+
+          void f(decltype(new auto{0}));
+
+        where we want to deduce the auto right away so that the parameter
+        is of type 'int *'.  */
+      auto cleanup = make_temp_override
+       (parser->auto_is_implicit_function_template_parm_p, false);
+
        /* Do not actually evaluate the expression.  */
        ++cp_unevaluated_operand;

@@ -24144,22 +24154,22 @@ cp_parser_type_id_1 (cp_parser *parser, cp_parser_flags flags,

          /* OK */;
        else
          {
-           location_t loc = type_specifier_seq.locations[ds_type_spec];
-           if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
+           if (!cp_parser_simulate_error (parser))
              {
-               if (!cp_parser_simulate_error (parser))
+               location_t loc = type_specifier_seq.locations[ds_type_spec];
+               if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
                  {
                    error_at (loc, "missing template arguments after %qT",
                              auto_node);
                    inform (DECL_SOURCE_LOCATION (tmpl), "%qD declared here",
                            tmpl);
                  }
+               else if (parser->in_template_argument_list_p)
+                 error_at (loc, "%qT not permitted in template argument",
+                           auto_node);
+               else
+                 error_at (loc, "invalid use of %qT", auto_node);
              }
-           else if (parser->in_template_argument_list_p)
-             error_at (loc, "%qT not permitted in template argument",
-                       auto_node);
-           else
-             error_at (loc, "invalid use of %qT", auto_node);
            return error_mark_node;
          }
        }
@@ -24668,6 +24678,15 @@ cp_parser_parameter_declaration (cp_parser *parser,
                                &decl_specifiers,
                                &declares_class_or_enum);

+ /* [dcl.spec.auto.general]: "A placeholder-type-specifier of the form

+     type-constraint opt auto can be used as a decl-specifier of the
+     decl-specifier-seq of a parameter-declaration of a function declaration
+     or lambda-expression..." but we must not synthesize an implicit template
+     type parameter in its declarator.  That is, in "void f(auto[auto{10}]);"
+     we want to synthesize only the first auto.  */
+  auto cleanup = make_temp_override
+    (parser->auto_is_implicit_function_template_parm_p, false);
+
    /* Complain about missing 'typename' or other invalid type names.  */
    if (!decl_specifiers.any_type_specifiers_p
        && cp_parser_parse_and_diagnose_invalid_type_name (parser))
@@ -32369,6 +32388,9 @@ cp_parser_sizeof_operand (cp_parser* parser, enum rid 
keyword)
      = parser->non_integral_constant_expression_p;
    parser->integral_constant_expression_p = false;

+ auto cleanup = make_temp_override

+    (parser->auto_is_implicit_function_template_parm_p, false);
+
    /* Do not actually evaluate the expression.  */
    ++cp_unevaluated_operand;
    ++c_inhibit_evaluation_warnings;
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C 
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C
index 8fb8dfdeaf0..dbc9e8c732c 100644
--- a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-85713-2.C
@@ -2,6 +2,6 @@
  // { dg-do compile { target c++14 } }

auto l4 = [](auto v, auto (&array (int)) [5]) -> int { return v + array[0]; };

-auto l5 = [](auto v, auto (&array (auto)) [5]) -> int { return v + array[0]; };
+auto l5 = [](auto v, auto (&array (auto)) [5]) -> int { return v + array[0]; };     // { 
dg-error ".auto. parameter not permitted in this context" }
  auto l6 = [](auto v, auto (&array (int int)) [5]) -> int { return v + array[0]; };  // 
{ dg-error "two or more data types" }
  auto l7 = [](auto v, auto (&array (int auto)) [5]) -> int { return v + array[0]; };  // 
{ dg-error "two or more data types" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr60054.C 
b/gcc/testsuite/g++.dg/cpp1y/pr60054.C
index f51120f6425..0d4925afbf2 100644
--- a/gcc/testsuite/g++.dg/cpp1y/pr60054.C
+++ b/gcc/testsuite/g++.dg/cpp1y/pr60054.C
@@ -6,6 +6,6 @@ template<typename T> decltype(T{}) fooB(T);

void bar()

  {
-  fooA((auto*)0);  // { dg-error "invalid use" }
-  fooB((auto*)0);  // { dg-error "invalid use" }
+  fooA((auto*)0);  // { dg-error "expected" }
+  fooB((auto*)0);  // { dg-error "expected" }
  }
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr60332.C 
b/gcc/testsuite/g++.dg/cpp1y/pr60332.C
index e75ab8584e3..f3a7980b380 100644
--- a/gcc/testsuite/g++.dg/cpp1y/pr60332.C
+++ b/gcc/testsuite/g++.dg/cpp1y/pr60332.C
@@ -3,4 +3,5 @@

void foo();-auto f = (auto(*)())(&foo); // { dg-error "invalid" }

+auto f = (auto(*)())(&foo);  // { dg-error "expected" }
+// { dg-error "only available" "" { target c++20_down } .-1 }
diff --git a/gcc/testsuite/g++.dg/cpp23/auto-fncast7.C 
b/gcc/testsuite/g++.dg/cpp23/auto-fncast7.C
new file mode 100644
index 00000000000..763164f3e5b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp23/auto-fncast7.C
@@ -0,0 +1,9 @@
+// PR c++/103401
+// { dg-do compile { target c++23 } }
+
+void f(decltype(auto(0))) { }
+
+int main()
+{
+  f<int,int>(0); // { dg-error "no matching function" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp23/auto-fncast8.C 
b/gcc/testsuite/g++.dg/cpp23/auto-fncast8.C
new file mode 100644
index 00000000000..9fb7b9c2516
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp23/auto-fncast8.C
@@ -0,0 +1,42 @@
+// PR c++/103401
+// { dg-do compile { target c++23 } }
+
+void f1 (decltype(auto(0)));
+void f2 (decltype(auto{0}));
+void f3 (int = auto(42));
+void f4 (int = auto{42});
+void f5 (decltype(auto(0)) = auto(42));
+void f6 (auto (x));
+void f7 (int[auto(10)]);
+void f8 (int[auto{10}]);
+void f9 (auto[auto{10}]);
+void f10 (auto);
+void f11 (int x, decltype(x) y);
+void f12 (int[sizeof(auto{10})]);
+void f13 (int[sizeof(auto(10))]);
+void f14 (int[__extension__ alignof(auto{10})]);
+void f15 (int[__extension__ alignof(auto(10))]);
+
+void
+g ()
+{
+  int a[2];
+  f1 (1);
+  f2 (1);
+  f3 ();
+  f3 (1);
+  f4 ();
+  f4 (1);
+  f5 ();
+  f5 (1);
+  f6 ('a');
+  f7 (&a[0]);
+  f8 (&a[0]);
+  f9 (&a[0]);
+  f10 (1);
+  f11 (1, 2);
+  f12 (&a[0]);
+  f13 (&a[0]);
+  f14 (&a[0]);
+  f15 (&a[0]);
+}
diff --git a/gcc/testsuite/g++.dg/cpp23/auto-fncast9.C 
b/gcc/testsuite/g++.dg/cpp23/auto-fncast9.C
new file mode 100644
index 00000000000..12a0dcece75
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp23/auto-fncast9.C
@@ -0,0 +1,17 @@
+// PR c++/103401
+// { dg-do compile { target c++23 } }
+
+void f1(decltype(new auto{0}));
+void f2(decltype(new int{0}));
+
+void
+g ()
+{
+  int i;
+  void f3(decltype(new auto{0}));
+  void f4(decltype(new int{0}));
+  f1 (&i);
+  f2 (&i);
+  f3 (&i);
+  f4 (&i);
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C
index 025bbf3bb93..75f8e40ca58 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-2.C
@@ -4,9 +4,9 @@
  template <typename T>
  void foo1(T& t) {
    typename T::template C<void> tcv = t;
-  typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { 
target c++20 } }
-  T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
-  (typename T::template D<auto> (t)); // { dg-error "invalid|not 
permitted|unable" }
+  typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
+  T::template C<auto>::f (tcv, u); // { dg-error "" }
+  (typename T::template D<auto> (t)); // { dg-error "" }
  // { dg-warning "only available" "" { target c++17_down } .-1 }
  }

@@ -23,9 +23,9 @@ struct T1 {

  template <typename T>
  void foo2(T& t) {
    typename T::template C<void> tcv = t;
-  typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { 
target c++20 } }
-  T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
-  T::template D<auto> (t); // { dg-error "invalid|not permitted" }
+  typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
+  T::template C<auto>::f (tcv, u); // { dg-error "" }
+  T::template D<auto> (t); // { dg-error "" }
  }

struct T2 {

diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C
index 80a388462eb..1c1a41c0fa2 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979-3.C
@@ -8,9 +8,9 @@
  template <typename T>
  void foo1(T& t) {
    typename T::template C<void> tcv = t;
-  typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { 
target c++20 } }
-  T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
-  (typename T::template D<auto> (t)); // { dg-error "invalid|not permitted|no 
class" }
+  typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
+  T::template C<auto>::f (tcv, u); // { dg-error "" }
+  (typename T::template D<auto> (t)); // { dg-error "" }
  // { dg-warning "only available" "" { target c++17_down } .-1 }
  }

@@ -27,9 +27,9 @@ struct T1 {

  template <typename T>
  void foo2(T& t) {
    typename T::template C<void> tcv = t;
-  typename T::template C<auto> u = tcv; // { dg-error "not permitted" "" { 
target c++20 } }
-  T::template C<auto>::f (tcv, u); // { dg-error "incomplete|not permitted" }
-  T::template D<auto> (t); // { dg-error "yields a type|not permitted" }
+  typename T::template C<auto> u = tcv; // { dg-error "" "" { target c++20 } }
+  T::template C<auto>::f (tcv, u); // { dg-error "" }
+  T::template D<auto> (t); // { dg-error "" }
  }

struct T2 {

diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C
index 81555eb4554..a83601527bb 100644
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr84979.C
@@ -5,5 +5,5 @@ template<typename> void foo() {}

void bar()

  {
-  foo<auto>(); // { dg-error "not permitted|invalid|no matching function" }
+  foo<auto>(); // { dg-error "" }
  }

base-commit: b77968a70537429b4f548f90c369d26e6b6943cc

Re: [PATCH v4] c++: Handle auto(x) in parameter-declaration-clause [PR103401]

Reply via email to