* [PATCH] c++/modules: Restrict partitions when parsing import declarations [PR110808]
@ 2023-11-14 6:24 Nathaniel Shead
2023-11-23 17:11 ` Nathan Sidwell
0 siblings, 1 reply; 6+ messages in thread
From: Nathaniel Shead @ 2023-11-14 6:24 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell
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 <nathanieloshead@gmail.com>
---
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 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++/modules: Restrict partitions when parsing import declarations [PR110808]
2023-11-14 6:24 [PATCH] c++/modules: Restrict partitions when parsing import declarations [PR110808] Nathaniel Shead
@ 2023-11-23 17:11 ` Nathan Sidwell
2023-11-24 11:32 ` [PATCH v2] c++: Follow module grammar more closely [PR110808] Nathaniel Shead
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Sidwell @ 2023-11-23 17:11 UTC (permalink / raw)
To: Nathaniel Shead, gcc-patches; +Cc: Jason Merrill
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 <nathanieloshead@gmail.com>
> ---
> 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 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;
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] c++: Follow module grammar more closely [PR110808]
2023-11-23 17:11 ` Nathan Sidwell
@ 2023-11-24 11:32 ` Nathaniel Shead
2023-12-16 11:03 ` Nathaniel Shead
2024-01-02 22:50 ` Nathaniel Shead
0 siblings, 2 replies; 6+ messages in thread
From: Nathaniel Shead @ 2023-11-24 11:32 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches, Jason Merrill
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 <nathanieloshead@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] c++: Follow module grammar more closely [PR110808]
2023-11-24 11:32 ` [PATCH v2] c++: Follow module grammar more closely [PR110808] Nathaniel Shead
@ 2023-12-16 11:03 ` Nathaniel Shead
2024-01-06 22:23 ` Nathan Sidwell
2024-01-02 22:50 ` Nathaniel Shead
1 sibling, 1 reply; 6+ messages in thread
From: Nathaniel Shead @ 2023-12-16 11:03 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches, Jason Merrill
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638089.html
On Fri, Nov 24, 2023 at 10:32:13PM +1100, Nathaniel Shead wrote:
> 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 <nathanieloshead@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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Follow module grammar more closely [PR110808]
2023-11-24 11:32 ` [PATCH v2] c++: Follow module grammar more closely [PR110808] Nathaniel Shead
2023-12-16 11:03 ` Nathaniel Shead
@ 2024-01-02 22:50 ` Nathaniel Shead
1 sibling, 0 replies; 6+ messages in thread
From: Nathaniel Shead @ 2024-01-02 22:50 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches, Jason Merrill
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638089.html.
On Fri, Nov 24, 2023 at 10:32:13PM +1100, Nathaniel Shead wrote:
> 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 <nathanieloshead@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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Follow module grammar more closely [PR110808]
2023-12-16 11:03 ` Nathaniel Shead
@ 2024-01-06 22:23 ` Nathan Sidwell
0 siblings, 0 replies; 6+ messages in thread
From: Nathan Sidwell @ 2024-01-06 22:23 UTC (permalink / raw)
To: Nathaniel Shead; +Cc: gcc-patches, Jason Merrill
This is ok -- sorry, I thought I'd already acked this
On 12/16/23 06:03, Nathaniel Shead wrote:
> Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638089.html
>
> On Fri, Nov 24, 2023 at 10:32:13PM +1100, Nathaniel Shead wrote:
>> 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 <nathanieloshead@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
>>
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-06 22:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-14 6:24 [PATCH] c++/modules: Restrict partitions when parsing import declarations [PR110808] Nathaniel Shead
2023-11-23 17:11 ` Nathan Sidwell
2023-11-24 11:32 ` [PATCH v2] c++: Follow module grammar more closely [PR110808] Nathaniel Shead
2023-12-16 11:03 ` Nathaniel Shead
2024-01-06 22:23 ` Nathan Sidwell
2024-01-02 22:50 ` Nathaniel Shead
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).