On Thu, Nov 23, 2023 at 12:11:58PM -0500, Nathan Sidwell wrote:
> 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.
>
As I was attempting to rewrite the comments I ended up splitting up the
work that was being done by cp_parser_module_name a lot to better match
the grammar, and also caught a few other segfaults that were occurring
along the way.
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
This patch cleans up the parsing of module-declarations and
import-declarations to more closely follow the grammar defined by the
standard.
For instance, 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.
Additionally, we sometimes allow statements like 'import :X' or
'module :X' even when not in a named module, and this causes segfaults,
so we disallow this too.
PR c++/110808
gcc/cp/ChangeLog:
* parser.cc (cp_parser_module_name): Rewrite to handle
module-names and module-partitions independently.
(cp_parser_module_partition): New function.
(cp_parser_module_declaration): Parse module partitions
explicitly. Don't change state if parsing module decl failed.
(cp_parser_import_declaration): Handle different kinds of
import-declarations locally.
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/mod-invalid-1.C: New test.
* 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 <nathanielosh...@gmail.com>
---
gcc/cp/parser.cc | 100 ++++++++++++-------
gcc/testsuite/g++.dg/modules/mod-invalid-1.C | 7 ++
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 +-
7 files changed, 95 insertions(+), 36 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/mod-invalid-1.C
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 f6d088bc73f..20bd8d45a08 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14853,58 +14853,64 @@ cp_parser_already_scoped_statement (cp_parser*
parser, bool *if_p,
/* Modules */
-/* Parse a module-name,
- identifier
- module-name . identifier
- header-name
+/* Parse a module-name or module-partition.
- Returns a pointer to module object, NULL. */
+ module-name:
+ module-name-qualifier [opt] identifier
-static module_state *
-cp_parser_module_name (cp_parser *parser)
-{
- cp_token *token = cp_lexer_peek_token (parser->lexer);
- if (token->type == CPP_HEADER_NAME)
- {
- cp_lexer_consume_token (parser->lexer);
+ module-partition:
+ : module-name-qualifier [opt] identifier
- return get_module (token->u.value);
- }
+ module-name-qualifier:
+ identifier .
+ module-name-qualifier identifier .
- module_state *parent = NULL;
- bool partitioned = false;
- if (token->type == CPP_COLON && named_module_p ())
- {
- partitioned = true;
- cp_lexer_consume_token (parser->lexer);
- }
+ Returns a pointer to the module object, or NULL on failure.
+ For PARTITION_P, PARENT is the module this is a partition of. */
+
+static module_state *
+cp_parser_module_name (cp_parser *parser, bool partition_p = false,
+ module_state *parent = NULL)
+{
+ if (partition_p
+ && cp_lexer_consume_token (parser->lexer)->type != CPP_COLON)
+ return NULL;
for (;;)
{
if (cp_lexer_peek_token (parser->lexer)->type != CPP_NAME)
{
- cp_parser_error (parser, "expected module-name");
- break;
+ if (partition_p)
+ cp_parser_error (parser, "expected module-partition");
+ else
+ cp_parser_error (parser, "expected module-name");
+ return NULL;
}
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)
- partitioned = true;
- else if (token->type != CPP_DOT)
+ parent = get_module (name, parent, partition_p);
+ if (cp_lexer_peek_token (parser->lexer)->type != CPP_DOT)
break;
cp_lexer_consume_token (parser->lexer);
- }
+ }
return parent;
}
+/* Parse a module-partition. Defers to cp_parser_module_name. */
+
+static module_state *
+cp_parser_module_partition (cp_parser *parser, module_state *parent = NULL)
+{
+ return cp_parser_module_name (parser, /*partition_p=*/true, parent);
+}
+
/* Named module-declaration
__module ; PRAGMA_EOL
- __module private ; PRAGMA_EOL (unimplemented)
- [__export] __module module-name attr-spec-seq-opt ; PRAGMA_EOL
+ __module : private ; PRAGMA_EOL (unimplemented)
+ [__export] __module module-name module-partition [opt]
+ attr-spec-seq-opt ; PRAGMA_EOL
*/
static module_parse
@@ -14962,9 +14968,12 @@ cp_parser_module_declaration (cp_parser *parser,
module_parse mp_state,
else
{
module_state *mod = cp_parser_module_name (parser);
+ if (mod && cp_lexer_peek_token (parser->lexer)->type == CPP_COLON)
+ mod = cp_parser_module_partition (parser, mod);
tree attrs = cp_parser_attributes_opt (parser);
- mp_state = MP_PURVIEW_IMPORTS;
+ if (mod)
+ mp_state = MP_PURVIEW_IMPORTS;
if (!mod || !cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON))
goto skip_eol;
@@ -14976,7 +14985,10 @@ cp_parser_module_declaration (cp_parser *parser,
module_parse mp_state,
}
/* Import-declaration
- [__export] __import module-name attr-spec-seq-opt ; PRAGMA_EOL */
+ __import module-name attr-spec-seq-opt ; PRAGMA_EOL
+ __import module-partition attr-spec-seq-opt ; PRAGMA_EOL
+ __import header-name attr-spec-seq-opt ; PRAGMA_EOL
+*/
static void
cp_parser_import_declaration (cp_parser *parser, module_parse mp_state,
@@ -15004,7 +15016,27 @@ cp_parser_import_declaration (cp_parser *parser,
module_parse mp_state,
}
else
{
- module_state *mod = cp_parser_module_name (parser);
+ module_state *mod = NULL;
+ cp_token *next = cp_lexer_peek_token (parser->lexer);
+ if (next->type == CPP_HEADER_NAME)
+ {
+ cp_lexer_consume_token (parser->lexer);
+ mod = get_module (next->u.value);
+ }
+ else if (next->type == CPP_COLON)
+ {
+ /* An import specifying a module-partition shall only appear after the
+ module-declaration in a module unit: [module.import]/4. */
+ if (named_module_p ()
+ && (mp_state == MP_PURVIEW_IMPORTS
+ || mp_state == MP_PRIVATE_IMPORTS))
+ mod = cp_parser_module_partition (parser);
+ else
+ error_at (next->location, "import specifying a module-partition"
+ " must appear after a named module-declaration");
+ }
+ else
+ mod = cp_parser_module_name (parser);
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/mod-invalid-1.C
b/gcc/testsuite/g++.dg/modules/mod-invalid-1.C
new file mode 100644
index 00000000000..fadaba0b560
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/mod-invalid-1.C
@@ -0,0 +1,7 @@
+// { dg-additional-options "-fmodules-ts" }
+
+module;
+
+module :foo; // { dg-error "expected module-name" }
+
+import :foo; // { dg-error "import specifying a module-partition must appear
after a named module-declaration" }
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 00000000000..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 00000000000..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 00000000000..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
+++ b/gcc/testsuite/g++.dg/modules/part-hdr-1_c.C
@@ -2,4 +2,4 @@
// { dg-module-cmi {mod} }
export module mod;
-import mod:impl;
+import :impl;
diff --git a/gcc/testsuite/g++.dg/modules/part-mac-1_c.C
b/gcc/testsuite/g++.dg/modules/part-mac-1_c.C
index 78a53d2fda3..db57adcef44 100644
--- a/gcc/testsuite/g++.dg/modules/part-mac-1_c.C
+++ b/gcc/testsuite/g++.dg/modules/part-mac-1_c.C
@@ -2,4 +2,4 @@
// { dg-module-cmi {mod} }
export module mod;
-import mod:impl;
+import :impl;
--
2.42.0
