| Message ID | 656089c2.620a0220.3437d.6417@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] c++: Follow module grammar more closely [PR110808] | expand |
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 >
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 >
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 >>
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;