Re: [PATCH] c++/modules: Restrict partitions when parsing import declarations [PR110808]
On 11/14/23 01:24, Nathaniel Shead wrote:
I'll also note that the comments above the parsing functions here no
longer exactly match with the grammar in the standard, should they be
updated as well?
please.
Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
access.
-- >8 --
Currently we allow declarations like 'import A:B', even from an
unrelated source file (not part of module A), which causes errors in
merging declarations. However, the syntax in [module.import] doesn't
even allow this form of import, so this patch prevents this from
parsing at all and avoids the error that way.
ok, I messed that one right up :)
PR c++/110808
gcc/cp/ChangeLog:
* parser.cc (cp_parser_module_name): Add param import_p.
Disallow partitions after module-name for import decls.
(cp_parser_module_declaration): Pass import_p = false.
(cp_parser_import_declaration): Pass import_p = true.
gcc/testsuite/ChangeLog:
* g++.dg/modules/part-hdr-1_c.C: Fix syntax.
* g++.dg/modules/part-mac-1_c.C: Likewise.
* g++.dg/modules/part-8_a.C: New test.
* g++.dg/modules/part-8_b.C: New test.
* g++.dg/modules/part-8_c.C: New test.
Signed-off-by: Nathaniel Shead
---
gcc/cp/parser.cc| 11 ++-
gcc/testsuite/g++.dg/modules/part-8_a.C | 6 ++
gcc/testsuite/g++.dg/modules/part-8_b.C | 6 ++
gcc/testsuite/g++.dg/modules/part-8_c.C | 8
gcc/testsuite/g++.dg/modules/part-hdr-1_c.C | 2 +-
gcc/testsuite/g++.dg/modules/part-mac-1_c.C | 2 +-
6 files changed, 28 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/part-8_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-8_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-8_c.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5116bcb78f6..72a5c52313d 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14858,10 +14858,11 @@ cp_parser_already_scoped_statement (cp_parser*
parser, bool *if_p,
module-name . identifier
header-name
- Returns a pointer to module object, NULL. */
+ Returns a pointer to module object, or NULL. Also parses an optional
+ module-partition after the module-name unless IMPORT_P is true. */
static module_state *
-cp_parser_module_name (cp_parser *parser)
+cp_parser_module_name (cp_parser *parser, bool import_p)
{
cp_token *token = cp_lexer_peek_token (parser->lexer);
if (token->type == CPP_HEADER_NAME)
@@ -14890,7 +14891,7 @@ cp_parser_module_name (cp_parser *parser)
tree name = cp_lexer_consume_token (parser->lexer)->u.value;
parent = get_module (name, parent, partitioned);
token = cp_lexer_peek_token (parser->lexer);
- if (!partitioned && token->type == CPP_COLON)
+ if (!partitioned && !import_p && token->type == CPP_COLON)
partitioned = true;
else if (token->type != CPP_DOT)
break;
@@ -14961,7 +14962,7 @@ cp_parser_module_declaration (cp_parser *parser,
module_parse mp_state,
}
else
{
- module_state *mod = cp_parser_module_name (parser);
+ module_state *mod = cp_parser_module_name (parser, /*import_p=*/false);
tree attrs = cp_parser_attributes_opt (parser);
mp_state = MP_PURVIEW_IMPORTS;
@@ -15004,7 +15005,7 @@ cp_parser_import_declaration (cp_parser *parser,
module_parse mp_state,
}
else
{
- module_state *mod = cp_parser_module_name (parser);
+ module_state *mod = cp_parser_module_name (parser, /*import_p=*/true);
tree attrs = cp_parser_attributes_opt (parser);
if (!mod || !cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON))
diff --git a/gcc/testsuite/g++.dg/modules/part-8_a.C
b/gcc/testsuite/g++.dg/modules/part-8_a.C
new file mode 100644
index 000..09f956ff36f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-8_a.C
@@ -0,0 +1,6 @@
+// PR c++/110808
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi group:tres }
+
+export module group:tres;
+int mul() { return 0; }
diff --git a/gcc/testsuite/g++.dg/modules/part-8_b.C
b/gcc/testsuite/g++.dg/modules/part-8_b.C
new file mode 100644
index 000..1ade029495c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-8_b.C
@@ -0,0 +1,6 @@
+// PR c++/110808
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi group }
+
+export module group;
+export import :tres;
diff --git a/gcc/testsuite/g++.dg/modules/part-8_c.C
b/gcc/testsuite/g++.dg/modules/part-8_c.C
new file mode 100644
index 000..2351f28f909
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-8_c.C
@@ -0,0 +1,8 @@
+// PR c++/110808
+// { dg-additional-options "-fmodules-ts" }
+
+import group:tres; // { dg-error "expected .;." }
+
+int main() {
+ return mul(); // { dg-error "not declared" }
+}
diff --git a/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C
[PATCH] c++/modules: Restrict partitions when parsing import declarations [PR110808]
I'll also note that the comments above the parsing functions here no
longer exactly match with the grammar in the standard, should they be
updated as well?
Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
access.
-- >8 --
Currently we allow declarations like 'import A:B', even from an
unrelated source file (not part of module A), which causes errors in
merging declarations. However, the syntax in [module.import] doesn't
even allow this form of import, so this patch prevents this from
parsing at all and avoids the error that way.
PR c++/110808
gcc/cp/ChangeLog:
* parser.cc (cp_parser_module_name): Add param import_p.
Disallow partitions after module-name for import decls.
(cp_parser_module_declaration): Pass import_p = false.
(cp_parser_import_declaration): Pass import_p = true.
gcc/testsuite/ChangeLog:
* g++.dg/modules/part-hdr-1_c.C: Fix syntax.
* g++.dg/modules/part-mac-1_c.C: Likewise.
* g++.dg/modules/part-8_a.C: New test.
* g++.dg/modules/part-8_b.C: New test.
* g++.dg/modules/part-8_c.C: New test.
Signed-off-by: Nathaniel Shead
---
gcc/cp/parser.cc| 11 ++-
gcc/testsuite/g++.dg/modules/part-8_a.C | 6 ++
gcc/testsuite/g++.dg/modules/part-8_b.C | 6 ++
gcc/testsuite/g++.dg/modules/part-8_c.C | 8
gcc/testsuite/g++.dg/modules/part-hdr-1_c.C | 2 +-
gcc/testsuite/g++.dg/modules/part-mac-1_c.C | 2 +-
6 files changed, 28 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/part-8_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-8_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-8_c.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5116bcb78f6..72a5c52313d 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14858,10 +14858,11 @@ cp_parser_already_scoped_statement (cp_parser*
parser, bool *if_p,
module-name . identifier
header-name
- Returns a pointer to module object, NULL. */
+ Returns a pointer to module object, or NULL. Also parses an optional
+ module-partition after the module-name unless IMPORT_P is true. */
static module_state *
-cp_parser_module_name (cp_parser *parser)
+cp_parser_module_name (cp_parser *parser, bool import_p)
{
cp_token *token = cp_lexer_peek_token (parser->lexer);
if (token->type == CPP_HEADER_NAME)
@@ -14890,7 +14891,7 @@ cp_parser_module_name (cp_parser *parser)
tree name = cp_lexer_consume_token (parser->lexer)->u.value;
parent = get_module (name, parent, partitioned);
token = cp_lexer_peek_token (parser->lexer);
- if (!partitioned && token->type == CPP_COLON)
+ if (!partitioned && !import_p && token->type == CPP_COLON)
partitioned = true;
else if (token->type != CPP_DOT)
break;
@@ -14961,7 +14962,7 @@ cp_parser_module_declaration (cp_parser *parser,
module_parse mp_state,
}
else
{
- module_state *mod = cp_parser_module_name (parser);
+ module_state *mod = cp_parser_module_name (parser, /*import_p=*/false);
tree attrs = cp_parser_attributes_opt (parser);
mp_state = MP_PURVIEW_IMPORTS;
@@ -15004,7 +15005,7 @@ cp_parser_import_declaration (cp_parser *parser,
module_parse mp_state,
}
else
{
- module_state *mod = cp_parser_module_name (parser);
+ module_state *mod = cp_parser_module_name (parser, /*import_p=*/true);
tree attrs = cp_parser_attributes_opt (parser);
if (!mod || !cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON))
diff --git a/gcc/testsuite/g++.dg/modules/part-8_a.C
b/gcc/testsuite/g++.dg/modules/part-8_a.C
new file mode 100644
index 000..09f956ff36f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-8_a.C
@@ -0,0 +1,6 @@
+// PR c++/110808
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi group:tres }
+
+export module group:tres;
+int mul() { return 0; }
diff --git a/gcc/testsuite/g++.dg/modules/part-8_b.C
b/gcc/testsuite/g++.dg/modules/part-8_b.C
new file mode 100644
index 000..1ade029495c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-8_b.C
@@ -0,0 +1,6 @@
+// PR c++/110808
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi group }
+
+export module group;
+export import :tres;
diff --git a/gcc/testsuite/g++.dg/modules/part-8_c.C
b/gcc/testsuite/g++.dg/modules/part-8_c.C
new file mode 100644
index 000..2351f28f909
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-8_c.C
@@ -0,0 +1,8 @@
+// PR c++/110808
+// { dg-additional-options "-fmodules-ts" }
+
+import group:tres; // { dg-error "expected .;." }
+
+int main() {
+ return mul(); // { dg-error "not declared" }
+}
diff --git a/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C
b/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C
index 78a53d2fda3..db57adcef44 100644
--- a/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C
+++
