On 9/17/24 8:14 AM, Simon Martin wrote:
The invalid test case in this PR highlights a bad interaction between
the tentative_firewall and error recovery in cp_parser_decltype: the
firewall makes cp_parser_skip_to_closing_parenthesis a no-op, and the
parser does not make any progress, running "forever".

This patch calls cp_parser_commit_to_tentative_parse before initiating
error recovery.

Successfully tested on x86_64-pc-linux-gnu.

        PR c++/114858

gcc/cp/ChangeLog:

        * parser.cc (cp_parser_decltype): Commit tentative parse before
        initiating error recovery.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp0x/decltype10.C: Adjust test expectation.
        * g++.dg/cpp2a/pr114858.C: New test.
---
  gcc/cp/parser.cc                        |  3 +++
  gcc/testsuite/g++.dg/cpp0x/decltype10.C |  2 ++
  gcc/testsuite/g++.dg/cpp2a/pr114858.C   | 25 +++++++++++++++++++++++++
  3 files changed, 30 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/pr114858.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 4dd9474cf60..3a7c5ffe4c8 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -17508,6 +17508,9 @@ cp_parser_decltype (cp_parser *parser)
    /* Parse to the closing `)'.  */
    if (expr == error_mark_node || !parens.require_close (parser))
      {
+      /* Commit to the tentative_firewall so we actually skip to the closing
+        parenthesis.  */
+      cp_parser_commit_to_tentative_parse (parser);

I don't think this is right.

Earlier in cp_parser_decltype I see

/* If in_declarator_p, a reparse as an expression might succeed (60361). Otherwise, commit now for better diagnostics. */
  if (cp_parser_uncommitted_to_tentative_parse_p (parser)
      && !parser->in_declarator_p)
    cp_parser_commit_to_topmost_tentative_parse (parser);

Here we're in a declarator, so we didn't commit at that point. And we still don't want to commit if parsing fails; as the comment says, when reparsing as an expression-statement it might work. Though there seems not to be a testcase for that...

In trying to come up with a testcase, I wrote this one that already fails because the error doesn't happen until after the decltype, so we memorize the wrong result:

struct Helper { Helper(int, ...); };
template <class T> struct C;
template<> struct C<char> {};
char A = 1;
Helper testFail(int(A), C<decltype(A)>{}); // { dg-bogus "C<int>" }

So in the long term we need to overhaul this code to handle reparsing even without a syntax error. But it's not a high priority.

Getting back to your patch, I think the problem is in cp_parser_simple_type_specifier:

    case RID_DECLTYPE:
/* Since DR 743, decltype can either be a simple-type-specifier by itself or begin a nested-name-specifier. Parsing it will replace it with a CPP_DECLTYPE, so just rewind and let the CPP_DECLTYPE handling below decide what to do. */
      cp_parser_decltype (parser);
      cp_lexer_set_token_position (parser->lexer, token);
      break;

This assumes that cp_parser_decltype will always succeed, which is wrong. We need to check whether the token actually became CPP_DECLTYPE and parser_error if not.

Jason

Reply via email to