On 5/23/24 19:58, Nathaniel Shead wrote:
On Thu, May 23, 2024 at 05:11:39PM -0400, Jason Merrill wrote:
On 5/23/24 10:54, Nathaniel Shead wrote:
Bootstrapped and regtested (so far just modules.exp and dg.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
-- >8 --
This fixes an ICE when a module directive is not given at global scope.
Although not explicitly mentioned, it seems implied from [basic.link] p1
and [module.global.frag] that a module-declaration must appear at the
global scope after preprocessing. Apart from this the patch also
slightly improves the errors given when accidentally using a module
control-line in other situations where it is not expected.
This could also come up with something like
int module;
int i =
module; // error, unexpected module directive
Adding a line break seems like confusing advice for this problem; rather,
they need to remove the line break before 'module'. And possibly add it in
somewhere else, but the problem is that 'module' is the first token on the
line. And if I put that in a namespace,
namespace A {
int module;
int i =
module; // error, unexpected module directive
}
the problem is the same, but we get a different diagnostic.
True.
FWIW I just used the same wording as in 'cp_parser_import_declaration';
my understanding is it's because you can disambiguate by adding a
newline after the 'module' itself:
int module;
int i =
module
;
is OK. But I'll update this message to be clearer.
I think I'd leave the "must be at global scope" diagnostic to
cp_parser_module_declaration, and assume that if we see a module keyword at
function scope it wasn't intended to be a module directive.
How about this then? Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK, thanks.
-- >8 --
This fixes an ICE when a module directive is not given at global scope.
Although not explicitly mentioned, it seems implied from [basic.link] p1
and [module.global.frag] that a module-declaration must appear at the
global scope after preprocessing. Apart from this the patch also
slightly improves the errors given when accidentally using a module
control-line in other situations where it is not expected.
PR c++/115200
gcc/cp/ChangeLog:
* parser.cc (cp_parser_error_1): Special-case unexpected module
directives for better diagnostics.
(cp_parser_module_declaration): Check that the module
declaration is at global scope.
(cp_parser_import_declaration): Sync error message with that in
cp_parser_error_1.
gcc/testsuite/ChangeLog:
* g++.dg/modules/mod-decl-1.C: Update error messages.
* g++.dg/modules/mod-decl-6.C: New test.
* g++.dg/modules/mod-decl-7.C: New test.
* g++.dg/modules/mod-decl-8.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/parser.cc | 26 ++++++++++++++++++++---
gcc/testsuite/g++.dg/modules/mod-decl-1.C | 8 ++++---
gcc/testsuite/g++.dg/modules/mod-decl-6.C | 11 ++++++++++
gcc/testsuite/g++.dg/modules/mod-decl-7.C | 11 ++++++++++
gcc/testsuite/g++.dg/modules/mod-decl-8.C | 14 ++++++++++++
5 files changed, 64 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/mod-decl-6.C
create mode 100644 gcc/testsuite/g++.dg/modules/mod-decl-7.C
create mode 100644 gcc/testsuite/g++.dg/modules/mod-decl-8.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 476ddc0d63a..779625144db 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -3230,6 +3230,19 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
return;
}
+ if (cp_token_is_module_directive (token))
+ {
+ auto_diagnostic_group d;
+ error_at (token->location, "unexpected module directive");
+ if (token->keyword != RID__EXPORT)
+ inform (token->location, "perhaps insert a line break after"
+ " %qs, or other disambiguation, to prevent this being"
+ " considered a module control-line",
+ (token->keyword == RID__MODULE) ? "module" : "import");
+ cp_parser_skip_to_pragma_eol (parser, token);
+ return;
+ }
+
/* If this is actually a conflict marker, report it as such. */
if (token->type == CPP_LSHIFT
|| token->type == CPP_RSHIFT
@@ -15135,12 +15148,19 @@ cp_parser_module_declaration (cp_parser *parser,
module_parse mp_state,
parser->lexer->in_pragma = true;
cp_token *token = cp_lexer_consume_token (parser->lexer);
+ tree scope = current_scope ();
if (flag_header_unit)
{
error_at (token->location,
"module-declaration not permitted in header-unit");
goto skip_eol;
}
+ else if (scope != global_namespace)
+ {
+ error_at (token->location, "module-declaration must be at global scope");
+ inform (DECL_SOURCE_LOCATION (scope), "scope opened here");
+ goto skip_eol;
+ }
else if (mp_state == MP_FIRST && !exporting
&& cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
{
@@ -15217,9 +15237,9 @@ cp_parser_import_declaration (cp_parser *parser,
module_parse mp_state,
error_at (token->location, "post-module-declaration"
" imports must be contiguous");
note_lexer:
- inform (token->location, "perhaps insert a line break, or other"
- " disambiguation, to prevent this being considered a"
- " module control-line");
+ inform (token->location, "perhaps insert a line break after"
+ " %<import%>, or other disambiguation, to prevent this"
+ " being considered a module control-line");
skip_eol:
cp_parser_skip_to_pragma_eol (parser, token);
}
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-1.C
b/gcc/testsuite/g++.dg/modules/mod-decl-1.C
index 23d34483dd7..df398b3ef50 100644
--- a/gcc/testsuite/g++.dg/modules/mod-decl-1.C
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-1.C
@@ -10,17 +10,19 @@ module foo.second; // { dg-error "only permitted as" }
namespace Foo
{
-module third; // { dg-error "only permitted as" }
+module third; // { dg-error "must be at global scope" }
}
struct Baz
{
- module forth; // { dg-error "expected" }
+ module forth; // { dg-error "unexpected module directive" }
+ // { dg-message "line break after .module." "" { target *-*-* } .-1 }
};
void Bink ()
{
- module fifth; // { dg-error "expected" }
+ module fifth; // { dg-error "unexpected module directive" }
+ // { dg-message "line break after .module." "" { target *-*-* } .-1 }
}
module a.; // { dg-error "only permitted as" }
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-6.C
b/gcc/testsuite/g++.dg/modules/mod-decl-6.C
new file mode 100644
index 00000000000..5fb342455e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-6.C
@@ -0,0 +1,11 @@
+// PR c++/115200
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi !M }
+
+module;
+
+namespace ns { // { dg-message "scope opened here" }
+
+export module M; // { dg-error "global scope" }
+
+}
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-7.C
b/gcc/testsuite/g++.dg/modules/mod-decl-7.C
new file mode 100644
index 00000000000..d25a1938044
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-7.C
@@ -0,0 +1,11 @@
+// PR c++/115200
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi !M }
+
+module;
+
+void foo() {
+
+export module M; // { dg-error "unexpected module directive" }
+
+}
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-8.C
b/gcc/testsuite/g++.dg/modules/mod-decl-8.C
new file mode 100644
index 00000000000..9f5f1de544a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-8.C
@@ -0,0 +1,14 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+
+struct module {};
+struct import {};
+
+void foo() {
+ module
+ x; // OK
+ import
+ y; // OK
+}
