On 5/4/22 16:03, Marek Polacek wrote:
This patch fixes the second half of 64679.  Here we issue a wrong
"redefinition of 'int x'" for the following:

   struct Bar {
     Bar(int, int, int);
   };

   int x = 1;
   Bar bar(int(x), int(x), int{x}); // #1

cp_parser_parameter_declaration_list does pushdecl every time it sees
a named parameter, so the second "int(x)" causes the error.  That's
premature, since this turns out to be a constructor call after the
third argument!

If the first parameter is parenthesized, we can't push until we've
established we're looking at a function declaration.  Therefore this
could be fixed by some kind of lookahead.  I thought about introducing
a lightweight variant of cp_parser_parameter_declaration_list that would
not have any side effects and would return as soon as it figures out
whether it's looking at a declaration or expression.  Since that would
require fairly nontrivial changes, I wanted something simpler.  Something
like delaying the pushdecl until we've reached the ')' following the
parameter-declaration-clause.  But that doesn't quite cut it: we must
have pushed the parameters before processing a default argument, as in:

   Bar bar(int(a), int(b), int c = sizeof(a));  // valid

I wondered how this would affect

  void f(int (i), decltype(i) j = 42);

interestingly, clang and EDG both reject this, but they accept

  void f(int (i), bool b = true, decltype(i) j = 42);

which suggests a similar implementation strategy.  MSVC accepts both.

After more trial and error than I'd be willing to admit I came up with
the following, which stashes parameters into a vector when parsing
tentatively, and only pushes when we're about to commit to any tentative
parse (that handles the default argument case) or we're at the end of
a parameter-declaration-clause followed by a ')' (so that we still diagnose
redefining a parameter).

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

        PR c++/64679

gcc/cp/ChangeLog:

        * parser.cc (push_pending_decls): New.
        (cp_parser_parameter_declaration_clause): Maintain a vector of
        parameters that haven't been pushed yet.  Push them at the end of
        a valid parameter-declaration-clause
        (cp_parser_parameter_declaration_list): Take a new auto_vec parameter.
        Pass it down to cp_parser_parameter_declaration.  Do not pushdecl
        while parsing tentatively.
        (cp_parser_parameter_declaration): Take a new auto_vec parameter.
        push_pending_decls when we're about to commit.
        (cp_parser_cache_defarg): Adjust the call to
        cp_parser_parameter_declaration_list.

gcc/testsuite/ChangeLog:

        * g++.dg/parse/ambig11.C: New test.
        * g++.dg/parse/ambig12.C: New test.
        * g++.dg/parse/ambig13.C: New test.
---
  gcc/cp/parser.cc                     | 76 ++++++++++++++++++++++++----
  gcc/testsuite/g++.dg/parse/ambig11.C | 39 ++++++++++++++
  gcc/testsuite/g++.dg/parse/ambig12.C | 12 +++++
  gcc/testsuite/g++.dg/parse/ambig13.C | 32 ++++++++++++
  4 files changed, 148 insertions(+), 11 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/parse/ambig11.C
  create mode 100644 gcc/testsuite/g++.dg/parse/ambig12.C
  create mode 100644 gcc/testsuite/g++.dg/parse/ambig13.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5fa743b5a8e..dd51b3a7b79 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -2391,9 +2391,9 @@ static void cp_parser_type_specifier_seq
  static tree cp_parser_parameter_declaration_clause
    (cp_parser *, cp_parser_flags);
  static tree cp_parser_parameter_declaration_list
-  (cp_parser *, cp_parser_flags);
+  (cp_parser *, cp_parser_flags, auto_vec<tree> *);
  static cp_parameter_declarator *cp_parser_parameter_declaration
-  (cp_parser *, cp_parser_flags, bool, bool *);
+  (cp_parser *, cp_parser_flags, bool, bool *, auto_vec<tree> * = nullptr);
  static tree cp_parser_default_argument
    (cp_parser *, bool);
  static void cp_parser_function_body
@@ -24438,6 +24438,27 @@ function_being_declared_is_template_p (cp_parser* 
parser)
          (current_class_type));
  }
+/* Push any pending named parameters we have seen in this function declaration
+   and stashed into PENDING_DECLS.  This mechanism is used for cases like
+
+     S foo(int(x), int(x), int{x});
+
+   where it's not clear if we're dealing with a constructor call or a function
+   declaration until we've seen the last argument which breaks it up.  Pushing
+   immediately would result in the redefinition error on the second 'x'.
+
+   We only have to do this if the first parameter is parenthesized.  */
+
+static void
+push_pending_decls (auto_vec<tree> *pending_decls)
+{
+  if (!pending_decls)
+    return;
+  for (tree p : pending_decls)
+    pushdecl (p);
+  pending_decls->truncate (0);
+}
+
  /* Parse a parameter-declaration-clause.
parameter-declaration-clause:
@@ -24494,8 +24515,12 @@ cp_parser_parameter_declaration_clause (cp_parser* 
parser,
        return explicit_void_list_node;
      }
+ /* A vector of parameters that haven't been pushed yet. */
+  auto_vec<tree> pending_decls;
+
    /* Parse the parameter-declaration-list.  */
-  parameters = cp_parser_parameter_declaration_list (parser, flags);
+  parameters = cp_parser_parameter_declaration_list (parser, flags,
+                                                    &pending_decls);
    /* If a parse error occurred while parsing the
       parameter-declaration-list, then the entire
       parameter-declaration-clause is erroneous.  */
@@ -24525,6 +24550,14 @@ cp_parser_parameter_declaration_clause (cp_parser* 
parser,
    else
      ellipsis_p = false;
+ /* A valid parameter-declaration-clause can only be followed by a ')'.
+     So it's time to push all the parameters we have seen now that we
+     know we have a valid declaration.  Note that here we may not have
+     committed yet, nor should we.  Pushing here will detect the error
+     of redefining a parameter.  */
+  if (cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_PAREN))
+    push_pending_decls (&pending_decls);
+
    /* Finish the parameter list.  */
    if (!ellipsis_p)
      parameters = chainon (parameters, void_list_node);
@@ -24539,23 +24572,25 @@ cp_parser_parameter_declaration_clause (cp_parser* 
parser,
       parameter-declaration-list , parameter-declaration
The parser flags FLAGS is used to control type-specifier parsing.
+   PENDING_DECLS is a vector of parameters that haven't been pushed yet.
Returns a representation of the parameter-declaration-list, as for
     cp_parser_parameter_declaration_clause.  However, the
     `void_list_node' is never appended to the list.  */
static tree
-cp_parser_parameter_declaration_list (cp_parser* parser, cp_parser_flags flags)
+cp_parser_parameter_declaration_list (cp_parser* parser,
+                                     cp_parser_flags flags,
+                                     auto_vec<tree> *pending_decls)
  {
    tree parameters = NULL_TREE;
    tree *tail = &parameters;
-  bool saved_in_unbraced_linkage_specification_p;
    int index = 0;
/* The special considerations that apply to a function within an
       unbraced linkage specifications do not apply to the parameters
       to the function.  */
-  saved_in_unbraced_linkage_specification_p
+  bool saved_in_unbraced_linkage_specification_p
      = parser->in_unbraced_linkage_specification_p;
    parser->in_unbraced_linkage_specification_p = false;
@@ -24570,7 +24605,8 @@ cp_parser_parameter_declaration_list (cp_parser* parser, cp_parser_flags flags)
        parameter
        = cp_parser_parameter_declaration (parser, flags,
                                           /*template_parm_p=*/false,
-                                          &parenthesized_p);
+                                          &parenthesized_p,
+                                          pending_decls);
/* We don't know yet if the enclosing context is unavailable or deprecated,
         so wait and deal with it in grokparms if appropriate.  */
@@ -24602,7 +24638,16 @@ cp_parser_parameter_declaration_list (cp_parser* 
parser, cp_parser_flags flags)
                               parameter->decl_specifiers.attributes,
                               0);
        if (DECL_NAME (decl))
-       decl = pushdecl (decl);
+       {
+         /* We cannot pushdecl while parsing tentatively because that may
+            have side effects and we cannot be sure yet if we are parsing
+            a declaration.  */
+         if (pending_decls
+             && cp_parser_uncommitted_to_tentative_parse_p (parser))
+           pending_decls->safe_push (decl);
+         else
+           decl = pushdecl (decl);
+       }
if (decl != error_mark_node)
        {
@@ -24713,6 +24758,8 @@ cp_parser_parameter_declaration_list (cp_parser* 
parser, cp_parser_flags flags)
     token encountered during the parsing of the assignment-expression
     is not interpreted as a greater-than operator.)
+ PENDING_DECLS is a vector of parameters that haven't been pushed yet.
+
     Returns a representation of the parameter, or NULL if an error
     occurs.  If PARENTHESIZED_P is non-NULL, *PARENTHESIZED_P is set to
     true iff the declarator is of the form "(p)".  */
@@ -24721,7 +24768,8 @@ static cp_parameter_declarator *
  cp_parser_parameter_declaration (cp_parser *parser,
                                 cp_parser_flags flags,
                                 bool template_parm_p,
-                                bool *parenthesized_p)
+                                bool *parenthesized_p,
+                                auto_vec<tree> *pending_decls)
  {
    int declares_class_or_enum;
    cp_decl_specifier_seq decl_specifiers;
@@ -24823,7 +24871,12 @@ cp_parser_parameter_declaration (cp_parser *parser,
                cp_parser_simulate_error (parser);
            }
          else
-           cp_parser_commit_to_tentative_parse (parser);
+           {
+             /* OK, so this *is* a declaration.  Push any parameters we have
+                seen; they may be used in default arguments, noexcept, etc.  */
+             push_pending_decls (pending_decls);
+             cp_parser_commit_to_tentative_parse (parser);
+           }
        }
        /* Parse the declarator.  */
        declarator_token_start = token;
@@ -34049,7 +34102,8 @@ cp_parser_cache_defarg (cp_parser *parser, bool nsdmi)
                  cp_lexer_consume_token (parser->lexer);
                  begin_scope (sk_function_parms, NULL_TREE);
                  tree t = cp_parser_parameter_declaration_list
-                           (parser, CP_PARSER_FLAGS_NONE);
+                           (parser, CP_PARSER_FLAGS_NONE,
+                            /*pending_decls*/nullptr);
                  if (t == error_mark_node)
                    error = true;
                  pop_bindings_and_leave_scope ();
diff --git a/gcc/testsuite/g++.dg/parse/ambig11.C 
b/gcc/testsuite/g++.dg/parse/ambig11.C
new file mode 100644
index 00000000000..51a586f40b0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/ambig11.C
@@ -0,0 +1,39 @@
+// PR c++/64679
+// { dg-do run { target c++11 } }
+
+struct Bar {
+  int a, b, c;
+  Bar(int a, int b, int c) : a(a), b(b), c(c) { }
+};
+
+void
+f ()
+{
+  Bar fn1(int(a), int(b), int c = sizeof(a));
+  Bar fn2(int(x), int(y), int(z)); // { dg-warning "function declaration" }
+  Bar fn3(int(x), int(y), int);
+  Bar fn4(int (*p)(int(x), int(y))); // { dg-warning "function declaration" }
+  Bar fn5(int (x), int (*p)(int(x), int(y)), int);
+}
+
+int
+main ()
+{
+  int x = 1;
+  // This ain't a decl.
+  Bar v1(int(x), int(x), int{x});
+  if (v1.a != 1 || v1.b != v1.a || v1.c != v1.a)
+    __builtin_abort ();
+  Bar v2(int(x), int(x), 1);
+  if (v2.a != 1 || v2.b != v2.a || v2.c != 1)
+    __builtin_abort ();
+  Bar v3(int(x), int(x), int(1));
+  if (v3.a != 1 || v3.b != v3.a || v3.c != 1)
+    __builtin_abort ();
+  Bar v4(int(1), int(x), int{x});
+  if (v4.a != 1 || v4.b != 1 || v4.c != 1)
+    __builtin_abort ();
+  Bar v5(int{x}, int(x), int{x});
+  if (v5.a != 1 || v5.b != v5.a || v5.c != v5.a)
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/parse/ambig12.C 
b/gcc/testsuite/g++.dg/parse/ambig12.C
new file mode 100644
index 00000000000..981f35f32a2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/ambig12.C
@@ -0,0 +1,12 @@
+// PR c++/64679
+
+struct Bar {
+  Bar (int, int, int);
+};
+
+void
+g ()
+{
+  Bar e1(int(x), int(x), int); // { dg-error "redefinition" }
+  Bar e2(int (*p)(int(x), int(x)), int); // { dg-error "redefinition" }
+}
diff --git a/gcc/testsuite/g++.dg/parse/ambig13.C 
b/gcc/testsuite/g++.dg/parse/ambig13.C
new file mode 100644
index 00000000000..5f6a3b934c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/ambig13.C
@@ -0,0 +1,32 @@
+// PR c++/64679
+// { dg-do compile { target c++11 } }
+
+struct Bar {
+  Bar (int, int, int);
+};
+
+template<typename T>
+void
+g ()
+{
+  int x = 1;
+  Bar v1(T(x), T(x), T{x});
+  Bar v2(T(x), T(x), T(1));
+}
+
+void
+invoke (Bar (*p)) noexcept(noexcept(*p))
+{
+}
+
+auto
+pmf (int (Bar::*p)) -> decltype(p)
+{
+  return nullptr;
+}
+
+void
+f ()
+{
+  g<int>();
+}

base-commit: c2e846b539bb932d7f68f7e6b3e401c361cc3bf3

Reply via email to