[PATCH] c++: wrong parse with functors [PR64679]

[PATCH] c++: wrong parse with functors [PR64679]

[Marek Polacek]

Consider

  struct F {
    F(int) {}
    F operator()(int) const { return *this; }
  };

and

  F(i)(0)(0);

where we're supposed to first call the constructor and then invoke
the operator() twice.  However, we parse this as an init-declarator:
"(i)" looks like a perfectly valid declarator, then we see an '(' and
think it must be an initializer, so we commit and we're toast.  My
fix is to look a little bit farther before deciding we've seen an
initializer.

This is only a half of c++/64679, the other part of the PR is unrelated:
there the problem is that we are calling pushdecl while parsing
tentatively (in cp_parser_parameter_declaration_list), which is bad.
I don't know how to fix it though, maybe move the pushdecl call to
grokparms?  Tricky :(.

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

	PR c++/64679

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_init_declarator): Properly handle a series of
	operator() calls, they are not part of an init-declarator.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/functor1.C: New test.
---
 gcc/cp/parser.cc                      | 25 ++++++++++++++++++++++++-
 gcc/testsuite/g++.dg/parse/functor1.C | 22 ++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/parse/functor1.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index a5cbb3e896f..6e2936b68ef 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -22636,11 +22636,34 @@ cp_parser_init_declarator (cp_parser* parser,
       return error_mark_node;
     }
 
-  /* An `=' or an `(', or an '{' in C++0x, indicates an initializer.  */
+  /* An `=' or an '{' in C++11, indicate an initializer.  An '(' may indicate
+     an initializer as well. */
   if (token->type == CPP_EQ
       || token->type == CPP_OPEN_PAREN
       || token->type == CPP_OPEN_BRACE)
     {
+      /* Don't get fooled into thinking that F(i)(1)(2) is an initializer.
+	 It isn't; it's an expression.  (Here '(i)' would have already been
+	 parsed as a declarator.)   */
+      if (token->type == CPP_OPEN_PAREN
+	  && cp_parser_uncommitted_to_tentative_parse_p (parser))
+	{
+	  cp_lexer_save_tokens (parser->lexer);
+	  cp_lexer_consume_token (parser->lexer);
+	  cp_parser_skip_to_closing_parenthesis (parser,
+						 /*recovering*/false,
+						 /*or_comma*/false,
+						 /*consume_paren*/true);
+	  /* If this is an initializer, only a ',' or ';' can follow: either
+	     we have another init-declarator, or we're at the end of an
+	     init-declarator-list which can only be followed by a ';'.  */
+	  bool ok = (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)
+		     || cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
+	  cp_lexer_rollback_tokens (parser->lexer);
+	  if (__builtin_expect (!ok, 0))
+	    /* Not an init-declarator.  */
+	    return error_mark_node;
+	}
       is_initialized = SD_INITIALIZED;
       initialization_kind = token->type;
       declarator->init_loc = token->location;
diff --git a/gcc/testsuite/g++.dg/parse/functor1.C b/gcc/testsuite/g++.dg/parse/functor1.C
new file mode 100644
index 00000000000..c014114c098
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/functor1.C
@@ -0,0 +1,22 @@
+// PR c++/64679
+// { dg-do run }
+
+struct F {
+  F(int) { }
+  F(int, int) { }
+  F operator()(int) const { return *this; }
+  F operator()(int, int) const { return *this; }
+};
+
+int main()
+{
+  // Init-declarators.
+  int i = 0;
+  int (j)(1);
+  // None of these is an init-declarator.
+  F(i)(1)(2);
+  F(i)(1, 2)(3);
+  F(i)(1)(2, 3);
+  F(i)(2)(3)(4)(5);
+  F(i, j)(1)(2)(3)(4)(5)(6);
+}

base-commit: 1cb220498e1f59021dab36c39c5d726e9f070c6a
-- 
2.35.1

Re: [PATCH] c++: wrong parse with functors [PR64679]

[Jason Merrill]

On 5/2/22 12:18, Marek Polacek wrote:
> Consider
> 
>    struct F {
>      F(int) {}
>      F operator()(int) const { return *this; }
>    };
> 
> and
> 
>    F(i)(0)(0);
> 
> where we're supposed to first call the constructor and then invoke
> the operator() twice.  However, we parse this as an init-declarator:
> "(i)" looks like a perfectly valid declarator, then we see an '(' and
> think it must be an initializer, so we commit and we're toast.

How vexing!

> My
> fix is to look a little bit farther before deciding we've seen an
> initializer.
> 
> This is only a half of c++/64679, the other part of the PR is unrelated:
> there the problem is that we are calling pushdecl while parsing
> tentatively (in cp_parser_parameter_declaration_list), which is bad.
> I don't know how to fix it though, maybe move the pushdecl call to
> grokparms?  Tricky :(.

Can we pop the parm decls when the tentative parse fails?

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

OK.

> 	PR c++/64679
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_init_declarator): Properly handle a series of
> 	operator() calls, they are not part of an init-declarator.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/parse/functor1.C: New test.
> ---
>   gcc/cp/parser.cc                      | 25 ++++++++++++++++++++++++-
>   gcc/testsuite/g++.dg/parse/functor1.C | 22 ++++++++++++++++++++++
>   2 files changed, 46 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/parse/functor1.C
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index a5cbb3e896f..6e2936b68ef 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -22636,11 +22636,34 @@ cp_parser_init_declarator (cp_parser* parser,
>         return error_mark_node;
>       }
>   
> -  /* An `=' or an `(', or an '{' in C++0x, indicates an initializer.  */
> +  /* An `=' or an '{' in C++11, indicate an initializer.  An '(' may indicate
> +     an initializer as well. */
>     if (token->type == CPP_EQ
>         || token->type == CPP_OPEN_PAREN
>         || token->type == CPP_OPEN_BRACE)
>       {
> +      /* Don't get fooled into thinking that F(i)(1)(2) is an initializer.
> +	 It isn't; it's an expression.  (Here '(i)' would have already been
> +	 parsed as a declarator.)   */
> +      if (token->type == CPP_OPEN_PAREN
> +	  && cp_parser_uncommitted_to_tentative_parse_p (parser))
> +	{
> +	  cp_lexer_save_tokens (parser->lexer);
> +	  cp_lexer_consume_token (parser->lexer);
> +	  cp_parser_skip_to_closing_parenthesis (parser,
> +						 /*recovering*/false,
> +						 /*or_comma*/false,
> +						 /*consume_paren*/true);
> +	  /* If this is an initializer, only a ',' or ';' can follow: either
> +	     we have another init-declarator, or we're at the end of an
> +	     init-declarator-list which can only be followed by a ';'.  */
> +	  bool ok = (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)
> +		     || cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
> +	  cp_lexer_rollback_tokens (parser->lexer);
> +	  if (__builtin_expect (!ok, 0))
> +	    /* Not an init-declarator.  */
> +	    return error_mark_node;
> +	}
>         is_initialized = SD_INITIALIZED;
>         initialization_kind = token->type;
>         declarator->init_loc = token->location;
> diff --git a/gcc/testsuite/g++.dg/parse/functor1.C b/gcc/testsuite/g++.dg/parse/functor1.C
> new file mode 100644
> index 00000000000..c014114c098
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/functor1.C
> @@ -0,0 +1,22 @@
> +// PR c++/64679
> +// { dg-do run }
> +
> +struct F {
> +  F(int) { }
> +  F(int, int) { }
> +  F operator()(int) const { return *this; }
> +  F operator()(int, int) const { return *this; }
> +};
> +
> +int main()
> +{
> +  // Init-declarators.
> +  int i = 0;
> +  int (j)(1);
> +  // None of these is an init-declarator.
> +  F(i)(1)(2);
> +  F(i)(1, 2)(3);
> +  F(i)(1)(2, 3);
> +  F(i)(2)(3)(4)(5);
> +  F(i, j)(1)(2)(3)(4)(5)(6);
> +}
> 
> base-commit: 1cb220498e1f59021dab36c39c5d726e9f070c6a

Re: [PATCH] c++: wrong parse with functors [PR64679]

[Marek Polacek]

On Tue, May 03, 2022 at 04:43:05PM -0400, Jason Merrill via Gcc-patches wrote:
> On 5/2/22 12:18, Marek Polacek wrote:
> > Consider
> > 
> >    struct F {
> >      F(int) {}
> >      F operator()(int) const { return *this; }
> >    };
> > 
> > and
> > 
> >    F(i)(0)(0);
> > 
> > where we're supposed to first call the constructor and then invoke
> > the operator() twice.  However, we parse this as an init-declarator:
> > "(i)" looks like a perfectly valid declarator, then we see an '(' and
> > think it must be an initializer, so we commit and we're toast.
> 
> How vexing!

:-) Most vexing indeed!
 
> > My
> > fix is to look a little bit farther before deciding we've seen an
> > initializer.
> > 
> > This is only a half of c++/64679, the other part of the PR is unrelated:
> > there the problem is that we are calling pushdecl while parsing
> > tentatively (in cp_parser_parameter_declaration_list), which is bad.
> > I don't know how to fix it though, maybe move the pushdecl call to
> > grokparms?  Tricky :(.
> 
> Can we pop the parm decls when the tentative parse fails?

Unfortunately no, we'll have already given a hard error when that
happens.  About to send a patch where I describe the problem in
detail.

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

Thanks!

Marek