On Tue, Jan 27, 2026 at 10:31:26AM +0800, Jason Merrill wrote:
> On 1/26/26 10:52 PM, Marek Polacek wrote:
> > On Sat, Jan 24, 2026 at 05:25:06PM +0800, Jason Merrill wrote:
> > > On 1/22/26 6:17 AM, Marek Polacek wrote:
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > >
> > > > -- >8 --
> > > > In cp_parser_splice_spec_is_nns_p I didn't use saved_token_sentinel:
> > > > its rollback uses cp_lexer_previous_token so if we are the first token
> > > > in the file, there are no previous tokens so we crash.
> > > >
> > > > It would be simple to just use the _safe variant of
> > > > cp_lexer_previous_token
> > > > and be done with this. But that's not how this worked out: I saw a new
> > > > -fcompare-debug FAIL with pr104025.C. The problem is that at the end of
> > > > cp_parser_id_expression we have a saved_token_sentinel guarded by
> > > > warn_missing_template_keyword and in that spot lexer->buffer is NULL (so
> > > > cp_lexer_set_source_position_from_token would be skipped). pr104025.C
> > > > is compiled twice, the second time with "-w -fcompare-debug-second". So
> > > > the first time we don't _set_source_position back to where we were
> > > > after the
> > > > _skip_entire_template_parameter_list (lexer->buffer is NULL) and the
> > > > second
> > > > time we don't get to the saved_token_sentinel at all. That left us with
> > > > different columns in the location:
> > > >
> > > > "pr104025.C":16:16 vs "pr104025.C":16:12
> > > >
> > > > thus the -fcompare-debug FAIL. I assume we don't want -fcompare-debug
> > > > to ignore the column location.
> > >
> > > Agreed.
> > >
> > > > So this patch adds STS_ROLLBACK_SAFE to
> > > > use the _safe variant of cp_lexer_previous_token.
> > >
> > > How about if safe_previous_token returns null then we use the current
> > > token
> > > instead? I'd rather not add separate modes.
> >
> > I also don't like adding a new mode. But using the current token leads
> > to the same -fcompare-debug FAIL with pr104025.C:
> > "pr104025.C":16:14 vs "pr104025.C":16:12
> >
> > As before, once we don't do saved_token_sentinel at all, and the second time
> > we _set_source_position to something that doesn't match the previous
> > position.
>
> Ah, so the problem is that _safe_previous_ is incompatible with
> cp_lexer_new_from_tokens because the latter sets buffer to null. Perhaps
> instead it should set it properly and the condition in cp_lexer_destroy
> should check saved_type instead of buffer.
>
> Or we could just save input_location in saved_token_sentinel instead of
> trying to recover it.
Now that the splice parsing fixes are in, I'm coming back to this
with the latter point implemented:
Bootstrapped/regtested on ppc64le-pc-linux-gnu, ok for trunk?
-- >8 --
In cp_parser_splice_spec_is_nns_p I didn't use saved_token_sentinel:
its rollback uses cp_lexer_previous_token so if we are the first token
in the file, there are no previous tokens so we crash.
It would be simple to just use the _safe variant of cp_lexer_previous_token
and be done with this. But that's not how this worked out: I saw a new
-fcompare-debug FAIL with pr104025.C. The problem is that at the end of
cp_parser_id_expression we have a saved_token_sentinel guarded by
warn_missing_template_keyword and in that spot lexer->buffer is NULL (so
cp_lexer_set_source_position_from_token would be skipped). pr104025.C
is compiled twice, the second time with "-w -fcompare-debug-second". So
the first time we don't _set_source_position back to where we were after the
_skip_entire_template_parameter_list (lexer->buffer is NULL) and the second
time we don't get to the saved_token_sentinel at all. That left us with
different columns in the location:
"pr104025.C":16:16 vs "pr104025.C":16:12
thus the -fcompare-debug FAIL. I assume we don't want -fcompare-debug
to ignore the column location. So we could just save input_location in
saved_token_sentinel instead of trying to recover it. And then
cp_parser_splice_spec_is_nns_p can be simplified.
gcc/cp/ChangeLog:
* parser.cc (struct saved_token_sentinel): Save input_location.
(saved_token_sentinel::rollback): Restore input_location.
(cp_parser_splice_spec_is_nns_p): Use saved_token_sentinel. Refactor.
---
gcc/cp/parser.cc | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 986605e0cdc..8c46b260fff 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -1635,10 +1635,11 @@ struct saved_token_sentinel
{
cp_lexer *lexer;
unsigned len;
+ location_t loc;
saved_token_sentinel_mode mode;
saved_token_sentinel (cp_lexer *_lexer,
saved_token_sentinel_mode _mode = STS_COMMIT)
- : lexer (_lexer), mode (_mode)
+ : lexer (_lexer), loc (input_location), mode (_mode)
{
len = lexer->saved_tokens.length ();
cp_lexer_save_tokens (lexer);
@@ -1646,8 +1647,7 @@ struct saved_token_sentinel
void rollback ()
{
cp_lexer_rollback_tokens (lexer);
- cp_lexer_set_source_position_from_token
- (cp_lexer_previous_token (lexer));
+ input_location = loc;
mode = STS_DONOTHING;
}
~saved_token_sentinel ()
@@ -6523,18 +6523,9 @@ cp_parser_skip_entire_splice_expr (cp_parser *parser)
static bool
cp_parser_splice_spec_is_nns_p (cp_parser *parser)
{
- /* ??? It'd be nice to use saved_token_sentinel, but its rollback
- uses cp_lexer_previous_token, but we may be the first token in the
- file so there are no previous tokens. Sigh. */
- cp_lexer_save_tokens (parser->lexer);
-
- const bool ok = (cp_parser_skip_entire_splice_expr (parser)
- && cp_lexer_next_token_is (parser->lexer, CPP_SCOPE));
-
- /* Roll back the tokens we skipped. */
- cp_lexer_rollback_tokens (parser->lexer);
-
- return ok;
+ saved_token_sentinel toks (parser->lexer, STS_ROLLBACK);
+ return (cp_parser_skip_entire_splice_expr (parser)
+ && cp_lexer_next_token_is (parser->lexer, CPP_SCOPE));
}
/* Return true if the N-th token is '[:' and its closing ':]' is NOT
base-commit: fbf8cb97c5bcfd66936131b3cfe006b9878191ef
--
2.53.0
