On Thu, Jul 31, 2025 at 11:04:56AM -0400, Jason Merrill wrote:
> If the current backup handling is doing the wrong thing in this case, it
> seems better to fix it rather than add more earlier in the function. Would
> it be enough to move the existing CPP_PRAGMA_EOL handling into the
> not_module block?
I guess either it would need to duplicate the _cpp_backup_tokens_direct call
like in the patch below (keep the eol handling just in not_module block,
otherwise have just a simple call), or I can move the eol variable to the
start of function, initialize there to false and only set it to peek->type
== CPP_PRAGMA_EOL in the not_module block, then the backup handling can be
shared (attached patch). The second (attached) is certainly shorter.
Both were just tested on the single testcase, which one do you prefer (or
something different)?
2025-07-31 Jakub Jelinek <ja...@redhat.com>
PR c++/120845
libcpp/
* lex.cc (cpp_maybe_module_directive): Move backup handling into
the if substatements, in the not_module case as is, otherwise
just if (backup) _cpp_backup_tokens_direct (). Formatting fix.
gcc/testsuite/
* g++.dg/modules/cpp-21.C: New test.
--- libcpp/lex.cc.jj 2025-04-24 15:29:41.714893932 +0200
+++ libcpp/lex.cc 2025-07-31 19:39:24.549920693 +0200
@@ -3564,10 +3564,10 @@ cpp_maybe_module_directive (cpp_reader *
tokens. C++ keywords are not yet relevant. */
if (peek->type == CPP_NAME
|| peek->type == CPP_COLON
- || (header_count
- ? (peek->type == CPP_LESS
- || (peek->type == CPP_STRING && peek->val.str.text[0] != 'R')
- || peek->type == CPP_HEADER_NAME)
+ || (header_count
+ ? (peek->type == CPP_LESS
+ || (peek->type == CPP_STRING && peek->val.str.text[0] != 'R')
+ || peek->type == CPP_HEADER_NAME)
: peek->type == CPP_SEMICOLON))
{
pfile->state.pragma_allow_expansion = !CPP_OPTION (pfile, preprocessed);
@@ -3678,6 +3678,9 @@ cpp_maybe_module_directive (cpp_reader *
}
while (true);
}
+ if (backup)
+ /* Put the peeked tokens back. */
+ _cpp_backup_tokens_direct (pfile, backup);
}
else
{
@@ -3689,21 +3692,19 @@ cpp_maybe_module_directive (cpp_reader *
pfile->state.in_deferred_pragma = false;
/* Do not let this remain on. */
pfile->state.angled_headers = false;
- }
-
- /* In either case we want to backup the peeked tokens. */
- if (backup)
- {
- /* If we saw EOL, we should drop it, because this isn't a module
- control-line after all. */
- bool eol = peek->type == CPP_PRAGMA_EOL;
- if (!eol || backup > 1)
+ if (backup)
{
- /* Put put the peeked tokens back */
- _cpp_backup_tokens_direct (pfile, backup);
- /* But if the last one was an EOL, forget it. */
- if (eol)
- pfile->lookaheads--;
+ /* If we saw EOL, we should drop it, because this isn't a module
+ control-line after all. */
+ bool eol = peek->type == CPP_PRAGMA_EOL;
+ if (!eol || backup > 1)
+ {
+ /* Put the peeked tokens back. */
+ _cpp_backup_tokens_direct (pfile, backup);
+ /* But if the last one was an EOL, forget it. */
+ if (eol)
+ pfile->lookaheads--;
+ }
}
}
}
--- gcc/testsuite/g++.dg/modules/cpp-21.C.jj 2025-06-27 12:34:17.226227463
+0200
+++ gcc/testsuite/g++.dg/modules/cpp-21.C 2025-06-27 12:34:28.052088809
+0200
@@ -0,0 +1,8 @@
+// PR c++/120845
+// { dg-do compile }
+// { dg-additional-options "-fmodules" }
+
+export module pr120485
+ [[foobarbaz]];
+// { dg-error "expected ';' before end of line" "" { target *-*-* } .-2 }
+// { dg-warning "attribute ignored" "" { target *-*-* } .-2 }
Jakub
2025-07-31 Jakub Jelinek <ja...@redhat.com>
PR c++/120845
libcpp/
* lex.cc (cpp_maybe_module_directive): Move eol variable declaration
to the start of the function, initialize to false and only set it to
peek->type == CPP_PRAGMA_EOL in the not_module case. Formatting fix.
gcc/testsuite/
* g++.dg/modules/cpp-21.C: New test.
--- libcpp/lex.cc.jj 2025-04-24 15:29:41.714893932 +0200
+++ libcpp/lex.cc 2025-07-31 19:52:18.791832523 +0200
@@ -3505,6 +3505,7 @@ cpp_maybe_module_directive (cpp_reader *
cpp_token *keyword = peek;
cpp_hashnode *(&n_modules)[spec_nodes::M_HWM][2] =
pfile->spec_nodes.n_modules;
int header_count = 0;
+ bool eol = false;
/* Make sure the incoming state is as we expect it. This way we
can restore it using constants. */
@@ -3564,10 +3565,10 @@ cpp_maybe_module_directive (cpp_reader *
tokens. C++ keywords are not yet relevant. */
if (peek->type == CPP_NAME
|| peek->type == CPP_COLON
- || (header_count
- ? (peek->type == CPP_LESS
- || (peek->type == CPP_STRING && peek->val.str.text[0] != 'R')
- || peek->type == CPP_HEADER_NAME)
+ || (header_count
+ ? (peek->type == CPP_LESS
+ || (peek->type == CPP_STRING && peek->val.str.text[0] != 'R')
+ || peek->type == CPP_HEADER_NAME)
: peek->type == CPP_SEMICOLON))
{
pfile->state.pragma_allow_expansion = !CPP_OPTION (pfile, preprocessed);
@@ -3689,22 +3690,19 @@ cpp_maybe_module_directive (cpp_reader *
pfile->state.in_deferred_pragma = false;
/* Do not let this remain on. */
pfile->state.angled_headers = false;
+ /* If we saw EOL, we should drop it, because this isn't a module
+ control-line after all. */
+ eol = peek->type == CPP_PRAGMA_EOL;
}
/* In either case we want to backup the peeked tokens. */
- if (backup)
+ if (backup && (!eol || backup > 1))
{
- /* If we saw EOL, we should drop it, because this isn't a module
- control-line after all. */
- bool eol = peek->type == CPP_PRAGMA_EOL;
- if (!eol || backup > 1)
- {
- /* Put put the peeked tokens back */
- _cpp_backup_tokens_direct (pfile, backup);
- /* But if the last one was an EOL, forget it. */
- if (eol)
- pfile->lookaheads--;
- }
+ /* Put the peeked tokens back. */
+ _cpp_backup_tokens_direct (pfile, backup);
+ /* But if the last one was an EOL in the not_module case, forget it. */
+ if (eol)
+ pfile->lookaheads--;
}
}
--- gcc/testsuite/g++.dg/modules/cpp-21.C.jj 2025-06-27 12:34:17.226227463
+0200
+++ gcc/testsuite/g++.dg/modules/cpp-21.C 2025-06-27 12:34:28.052088809
+0200
@@ -0,0 +1,8 @@
+// PR c++/120845
+// { dg-do compile }
+// { dg-additional-options "-fmodules" }
+
+export module pr120485
+ [[foobarbaz]];
+// { dg-error "expected ';' before end of line" "" { target *-*-* } .-2 }
+// { dg-warning "attribute ignored" "" { target *-*-* } .-2 }
