[PR,c++/84610,84642] recover from implicit template parms gracefully

Message ID or8tb0b0ta.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [PR,c++/84610,84642] recover from implicit template parms gracefully
Related show

Commit Message

Alexandre Oliva March 10, 2018, 11:57 a.m.
If we get a parse error during an attempted fully implicit function
template parse, and need to skip to the end of the statement or block,
we may discard the function parms scope rather than the enclosing
injected implicit template parms scope.  If we rollback a tentative
parse and try something else, we'll no longer be in a function parms
scope, but rather in a template parms scope, but we may still attempt
to synthesize implicit template parms and then fail the assert that
checks we're in a function parms scope.

This patch introduces an alternative to
finish_fully_implicit_template_p, to be used during error recovery,
that floats the implicit template parm scope to the top so that it
gets discarded as we finish and discard the failed implicit template
data, while other scopes are retained as expected.  It also clears the
implicit template parser data as we finish the template, so that it
doesn't linger on referencing discarded or used scopes and parms.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?

While debugging this, I first tried another patch, that avoids the same
ICEs.  I thought this one was a more complete solution, and it renders
the other unnecessary, but I still though it might be useful to disable
auto->implicit_parm while parsing declarators, so as to avoid useless
processing.

for gcc/cp/ChangeLog

	PR c++/84610
	PR c++/84642
	* parser.c (abort_fully_implicit_template_p): New.
	(cp_parser_skip_to_end_of_statement): Use it.
	(cp_parser_skip_to_end_of_block_or_statement): Likewise.
	(finish_fully_implicit_template_p): Clear
	implicit_template_parms and implicit_template_scope.

for  gcc/testsuite/ChangeLog

	PR c++/84610
	PR c++/84642
	* g++.dg/cpp0x/pr84610.C: New.
	* g++.dg/cpp0x/pr84642.C: New.
---
 gcc/cp/parser.c                      |   38 ++++++++++++++++++++++++++++++++--
 gcc/testsuite/g++.dg/cpp0x/pr84610.C |    3 +++
 gcc/testsuite/g++.dg/cpp0x/pr84642.C |    3 +++
 3 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr84610.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr84642.C



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Comments

Jason Merrill March 20, 2018, 7:43 p.m. | #1
On Sat, Mar 10, 2018 at 6:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> If we get a parse error during an attempted fully implicit function

> template parse, and need to skip to the end of the statement or block,

> we may discard the function parms scope rather than the enclosing

> injected implicit template parms scope.  If we rollback a tentative

> parse and try something else, we'll no longer be in a function parms

> scope, but rather in a template parms scope, but we may still attempt

> to synthesize implicit template parms and then fail the assert that

> checks we're in a function parms scope.

>

> This patch introduces an alternative to

> finish_fully_implicit_template_p, to be used during error recovery,

> that floats the implicit template parm scope to the top so that it

> gets discarded as we finish and discard the failed implicit template

> data, while other scopes are retained as expected.  It also clears the

> implicit template parser data as we finish the template, so that it

> doesn't linger on referencing discarded or used scopes and parms.

>

> Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?


OK.

> While debugging this, I first tried another patch, that avoids the same

> ICEs.  I thought this one was a more complete solution, and it renders

> the other unnecessary, but I still though it might be useful to disable

> auto->implicit_parm while parsing declarators, so as to avoid useless

> processing.


Better, I think, to disable it while parsing attributes; we could
still encounter auto as a template argument in a declarator.

Jason
Alexandre Oliva March 20, 2018, 9:57 p.m. | #2
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote:

>> While debugging this, I first tried another patch, that avoids the same

>> ICEs.  I thought this one was a more complete solution, and it renders

>> the other unnecessary, but I still though it might be useful to disable

>> auto->implicit_parm while parsing declarators, so as to avoid useless

>> processing.


> Better, I think, to disable it while parsing attributes; we could

> still encounter auto as a template argument in a declarator.


How about this?  Regstrapping, ok to install if it passes?

Disable auto_is_implicit_function_template_parm_p while parsing attributes

for  gcc/cp/ChangeLog

	PR c++/84610
	PR c++/84642
	* parser.c (temp_override): New template class,
	generalizing...
	(cp_parser_parameter_declaration_clause): ... this cleanup.
	Use it.
	(cp_parser_gnu_attributes_opt): Use it.
	(cp_parser_std_attribute): Use it.
---
 gcc/cp/parser.c |   36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 5cc201604166..ce05615adfba 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -255,6 +255,21 @@ static bool cp_parser_omp_declare_reduction_exprs
 static void cp_finalize_oacc_routine
   (cp_parser *, tree, bool);
 
+template <typename T>
+class temp_override
+{
+  T& overridden_variable;
+  T saved_value;
+public:
+  temp_override(T& var) : overridden_variable (var), saved_value (var) {}
+  temp_override(T& var, T overrider)
+    : overridden_variable (var), saved_value (var)
+  {
+    overridden_variable = overrider;
+  }
+  ~temp_override() { overridden_variable = saved_value; }
+};
+
 /* Manifest constants.  */
 #define CP_LEXER_BUFFER_SIZE ((256 * 1024) / sizeof (cp_token))
 #define CP_SAVED_TOKEN_STACK 5
@@ -21187,16 +21202,9 @@ cp_parser_parameter_declaration_clause (cp_parser* parser)
   bool ellipsis_p;
   bool is_error;
 
-  struct cleanup {
-    cp_parser* parser;
-    int auto_is_implicit_function_template_parm_p;
-    ~cleanup() {
-      parser->auto_is_implicit_function_template_parm_p
-	= auto_is_implicit_function_template_parm_p;
-    }
-  } cleanup = { parser, parser->auto_is_implicit_function_template_parm_p };
-
-  (void) cleanup;
+  temp_override<bool> cleanup
+    (parser->auto_is_implicit_function_template_parm_p);
+  (void)cleanup;
 
   if (!processing_specialization
       && !processing_template_parmlist
@@ -24959,6 +24967,10 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)
 {
   tree attributes = NULL_TREE;
 
+  temp_override<bool> cleanup
+    (parser->auto_is_implicit_function_template_parm_p, false);
+  (void)cleanup;
+
   while (true)
     {
       cp_token *token;
@@ -25150,6 +25162,10 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
   tree attribute, attr_id = NULL_TREE, arguments;
   cp_token *token;
 
+  temp_override<bool> cleanup
+    (parser->auto_is_implicit_function_template_parm_p, false);
+  (void)cleanup;
+
   /* First, parse name of the attribute, a.k.a attribute-token.  */
 
   token = cp_lexer_peek_token (parser->lexer);


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill March 20, 2018, 10:06 p.m. | #3
On Tue, Mar 20, 2018 at 5:57 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>>> While debugging this, I first tried another patch, that avoids the same

>>> ICEs.  I thought this one was a more complete solution, and it renders

>>> the other unnecessary, but I still though it might be useful to disable

>>> auto->implicit_parm while parsing declarators, so as to avoid useless

>>> processing.

>

>> Better, I think, to disable it while parsing attributes; we could

>> still encounter auto as a template argument in a declarator.

>

> How about this?  Regstrapping, ok to install if it passes?

>

> Disable auto_is_implicit_function_template_parm_p while parsing attributes

>

> for  gcc/cp/ChangeLog

>

>         PR c++/84610

>         PR c++/84642

>         * parser.c (temp_override): New template class,

>         generalizing...

>         (cp_parser_parameter_declaration_clause): ... this cleanup.

>         Use it.

>         (cp_parser_gnu_attributes_opt): Use it.

>         (cp_parser_std_attribute): Use it.

> ---

>  gcc/cp/parser.c |   36 ++++++++++++++++++++++++++----------

>  1 file changed, 26 insertions(+), 10 deletions(-)

>

> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> index 5cc201604166..ce05615adfba 100644

> --- a/gcc/cp/parser.c

> +++ b/gcc/cp/parser.c

> @@ -255,6 +255,21 @@ static bool cp_parser_omp_declare_reduction_exprs

>  static void cp_finalize_oacc_routine

>    (cp_parser *, tree, bool);

>

> +template <typename T>

> +class temp_override

> +{

> +  T& overridden_variable;

> +  T saved_value;

> +public:

> +  temp_override(T& var) : overridden_variable (var), saved_value (var) {}

> +  temp_override(T& var, T overrider)

> +    : overridden_variable (var), saved_value (var)

> +  {

> +    overridden_variable = overrider;

> +  }

> +  ~temp_override() { overridden_variable = saved_value; }

> +};


Let's put this in cp-tree.h, with warning_sentinel.

> +  (void)cleanup;


There are lots of RAII variables without this explicit cast to void,
and they don't seem to have been a problem; let's drop it here as
well.

Jason
Alexandre Oliva March 21, 2018, 9:41 p.m. | #4
On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Tue, Mar 20, 2018 at 5:57 PM, Alexandre Oliva <aoliva@redhat.com> wrote:

>> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote:

> Let's put this in cp-tree.h, with warning_sentinel.


>> +  (void)cleanup;


> There are lots of RAII variables without this explicit cast to void,

> and they don't seem to have been a problem; let's drop it here as

> well.


Done, here's what passed regstrap on i686- and x86_64-linux-gnu last
night.  Ok to install?

Disable auto_is_implicit_function_template_parm_p while parsing attributes

for  gcc/cp/ChangeLog

	PR c++/84610
	PR c++/84642
	* cp-tree.h (temp_override): New template class, generalizing
	a cleanup that was only used...
	* parser.c (cp_parser_parameter_declaration_clause):
	... here for auto_is_implicit_function_template_parm_p.
	(cp_parser_gnu_attributes_opt): Use it here as well.
	(cp_parser_std_attribute): Likewise.
---
 gcc/cp/cp-tree.h |   19 +++++++++++++++++++
 gcc/cp/parser.c  |   18 ++++++++----------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c07aaa5781ac..c8f4bc43fa3c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1657,6 +1657,25 @@ struct warning_sentinel
   ~warning_sentinel() { flag = val; }
 };
 
+/* RAII sentinel that saves the value of a variable, optionally
+   overrides it right away, and restores its value when the sentinel
+   id destructed.  */
+
+template <typename T>
+class temp_override
+{
+  T& overridden_variable;
+  T saved_value;
+public:
+  temp_override(T& var) : overridden_variable (var), saved_value (var) {}
+  temp_override(T& var, T overrider)
+    : overridden_variable (var), saved_value (var)
+  {
+    overridden_variable = overrider;
+  }
+  ~temp_override() { overridden_variable = saved_value; }
+};
+
 /* The cached class binding level, from the most recently exited
    class, or NULL if none.  */
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6dcfae125b7b..34619293120b 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21196,16 +21196,8 @@ cp_parser_parameter_declaration_clause (cp_parser* parser)
   bool ellipsis_p;
   bool is_error;
 
-  struct cleanup {
-    cp_parser* parser;
-    int auto_is_implicit_function_template_parm_p;
-    ~cleanup() {
-      parser->auto_is_implicit_function_template_parm_p
-	= auto_is_implicit_function_template_parm_p;
-    }
-  } cleanup = { parser, parser->auto_is_implicit_function_template_parm_p };
-
-  (void) cleanup;
+  temp_override<bool> cleanup
+    (parser->auto_is_implicit_function_template_parm_p);
 
   if (!processing_specialization
       && !processing_template_parmlist
@@ -24968,6 +24960,9 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)
 {
   tree attributes = NULL_TREE;
 
+  temp_override<bool> cleanup
+    (parser->auto_is_implicit_function_template_parm_p, false);
+
   while (true)
     {
       cp_token *token;
@@ -25159,6 +25154,9 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
   tree attribute, attr_id = NULL_TREE, arguments;
   cp_token *token;
 
+  temp_override<bool> cleanup
+    (parser->auto_is_implicit_function_template_parm_p, false);
+
   /* First, parse name of the attribute, a.k.a attribute-token.  */
 
   token = cp_lexer_peek_token (parser->lexer);


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill March 22, 2018, 5:47 p.m. | #5
OK.

On Wed, Mar 21, 2018 at 5:41 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote:

>

>> On Tue, Mar 20, 2018 at 5:57 PM, Alexandre Oliva <aoliva@redhat.com> wrote:

>>> On Mar 20, 2018, Jason Merrill <jason@redhat.com> wrote:

>> Let's put this in cp-tree.h, with warning_sentinel.

>

>>> +  (void)cleanup;

>

>> There are lots of RAII variables without this explicit cast to void,

>> and they don't seem to have been a problem; let's drop it here as

>> well.

>

> Done, here's what passed regstrap on i686- and x86_64-linux-gnu last

> night.  Ok to install?

>

> Disable auto_is_implicit_function_template_parm_p while parsing attributes

>

> for  gcc/cp/ChangeLog

>

>         PR c++/84610

>         PR c++/84642

>         * cp-tree.h (temp_override): New template class, generalizing

>         a cleanup that was only used...

>         * parser.c (cp_parser_parameter_declaration_clause):

>         ... here for auto_is_implicit_function_template_parm_p.

>         (cp_parser_gnu_attributes_opt): Use it here as well.

>         (cp_parser_std_attribute): Likewise.

> ---

>  gcc/cp/cp-tree.h |   19 +++++++++++++++++++

>  gcc/cp/parser.c  |   18 ++++++++----------

>  2 files changed, 27 insertions(+), 10 deletions(-)

>

> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h

> index c07aaa5781ac..c8f4bc43fa3c 100644

> --- a/gcc/cp/cp-tree.h

> +++ b/gcc/cp/cp-tree.h

> @@ -1657,6 +1657,25 @@ struct warning_sentinel

>    ~warning_sentinel() { flag = val; }

>  };

>

> +/* RAII sentinel that saves the value of a variable, optionally

> +   overrides it right away, and restores its value when the sentinel

> +   id destructed.  */

> +

> +template <typename T>

> +class temp_override

> +{

> +  T& overridden_variable;

> +  T saved_value;

> +public:

> +  temp_override(T& var) : overridden_variable (var), saved_value (var) {}

> +  temp_override(T& var, T overrider)

> +    : overridden_variable (var), saved_value (var)

> +  {

> +    overridden_variable = overrider;

> +  }

> +  ~temp_override() { overridden_variable = saved_value; }

> +};

> +

>  /* The cached class binding level, from the most recently exited

>     class, or NULL if none.  */

>

> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c

> index 6dcfae125b7b..34619293120b 100644

> --- a/gcc/cp/parser.c

> +++ b/gcc/cp/parser.c

> @@ -21196,16 +21196,8 @@ cp_parser_parameter_declaration_clause (cp_parser* parser)

>    bool ellipsis_p;

>    bool is_error;

>

> -  struct cleanup {

> -    cp_parser* parser;

> -    int auto_is_implicit_function_template_parm_p;

> -    ~cleanup() {

> -      parser->auto_is_implicit_function_template_parm_p

> -       = auto_is_implicit_function_template_parm_p;

> -    }

> -  } cleanup = { parser, parser->auto_is_implicit_function_template_parm_p };

> -

> -  (void) cleanup;

> +  temp_override<bool> cleanup

> +    (parser->auto_is_implicit_function_template_parm_p);

>

>    if (!processing_specialization

>        && !processing_template_parmlist

> @@ -24968,6 +24960,9 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)

>  {

>    tree attributes = NULL_TREE;

>

> +  temp_override<bool> cleanup

> +    (parser->auto_is_implicit_function_template_parm_p, false);

> +

>    while (true)

>      {

>        cp_token *token;

> @@ -25159,6 +25154,9 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)

>    tree attribute, attr_id = NULL_TREE, arguments;

>    cp_token *token;

>

> +  temp_override<bool> cleanup

> +    (parser->auto_is_implicit_function_template_parm_p, false);

> +

>    /* First, parse name of the attribute, a.k.a attribute-token.  */

>

>    token = cp_lexer_peek_token (parser->lexer);

>

>

> --

> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/

> You must be the change you wish to see in the world. -- Gandhi

> Be Free! -- http://FSFLA.org/   FSF Latin America board member

> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index cdc623889732..e17507d7f6e0 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -2264,6 +2264,8 @@  static tree synthesize_implicit_template_parm
   (cp_parser *, tree);
 static tree finish_fully_implicit_template
   (cp_parser *, tree);
+static void abort_fully_implicit_template
+  (cp_parser *);
 
 /* Classes [gram.class] */
 
@@ -3585,7 +3587,7 @@  cp_parser_skip_to_end_of_statement (cp_parser* parser)
 
   /* Unwind generic function template scope if necessary.  */
   if (parser->fully_implicit_function_template_p)
-    finish_fully_implicit_template (parser, /*member_decl_opt=*/0);
+    abort_fully_implicit_template (parser);
 
   while (true)
     {
@@ -3675,7 +3677,7 @@  cp_parser_skip_to_end_of_block_or_statement (cp_parser* parser)
 
   /* Unwind generic function template scope if necessary.  */
   if (parser->fully_implicit_function_template_p)
-    finish_fully_implicit_template (parser, /*member_decl_opt=*/0);
+    abort_fully_implicit_template (parser);
 
   while (nesting_depth >= 0)
     {
@@ -39261,11 +39263,43 @@  finish_fully_implicit_template (cp_parser *parser, tree member_decl_opt)
   end_template_decl ();
 
   parser->fully_implicit_function_template_p = false;
+  parser->implicit_template_parms = 0;
+  parser->implicit_template_scope = 0;
   --parser->num_template_parameter_lists;
 
   return member_decl_opt;
 }
 
+/* Like finish_fully_implicit_template, but to be used in error
+   recovery, rearranging scopes so that we restore the state we had
+   before synthesize_implicit_template_parm inserted the implement
+   template parms scope.  */
+
+static void
+abort_fully_implicit_template (cp_parser *parser)
+{
+  cp_binding_level *return_to_scope = current_binding_level;
+
+  if (parser->implicit_template_scope
+      && return_to_scope != parser->implicit_template_scope)
+    {
+      cp_binding_level *child = return_to_scope;
+      for (cp_binding_level *scope = child->level_chain;
+	   scope != parser->implicit_template_scope;
+	   scope = child->level_chain)
+	child = scope;
+      child->level_chain = parser->implicit_template_scope->level_chain;
+      parser->implicit_template_scope->level_chain = return_to_scope;
+      current_binding_level = parser->implicit_template_scope;
+    }
+  else
+    return_to_scope = return_to_scope->level_chain;
+
+  finish_fully_implicit_template (parser, NULL);
+
+  gcc_assert (current_binding_level == return_to_scope);
+}
+
 /* Helper function for diagnostics that have complained about things
    being used with 'extern "C"' linkage.
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr84610.C b/gcc/testsuite/g++.dg/cpp0x/pr84610.C
new file mode 100644
index 000000000000..cc70748967bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr84610.C
@@ -0,0 +1,3 @@ 
+// { dg-do compile { target c++11 } }
+
+a (int &__attribute__ ((aligned(auto x)) y)); // { dg-error "parameter declaration|before|expected constructor" }
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr84642.C b/gcc/testsuite/g++.dg/cpp0x/pr84642.C
new file mode 100644
index 000000000000..5c6895edb970
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr84642.C
@@ -0,0 +1,3 @@ 
+// { dg-do compile { target c++11 } }
+
+a(int(const &&__attribute__((b(auto;))))) // { dg-error "parameter declaration|with no type|before|expected constructor" }



Reset auto_is_implicit_function_template_parm_p while parsing declarators

for gcc/cp/ChangeLog

	PR c++/84610
	PR c++/84642
	* parser.c (cp_parser_parameter_declaration): Reset
	auto_is_implicit_function_template_parm_p while parsing
	declarators.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9f135cb4cda1..387d58814538 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21520,6 +21520,10 @@  cp_parser_parameter_declaration (cp_parser *parser,
       return NULL;
     }
 
+  bool saved_auto_is_implicit_function_template_parm_p
+    = parser->auto_is_implicit_function_template_parm_p;
+  parser->auto_is_implicit_function_template_parm_p = false;
+
   /* Peek at the next token.  */
   token = cp_lexer_peek_token (parser->lexer);
 
@@ -21709,6 +21713,9 @@  cp_parser_parameter_declaration (cp_parser *parser,
 					decl_spec_token_start->location,
 					input_location);
 
+  parser->auto_is_implicit_function_template_parm_p
+    = saved_auto_is_implicit_function_template_parm_p;
+
   return make_parameter_declarator (&decl_specifiers,
 				    declarator,
 				    default_argument,