From 7f24446a3cf67d1346c78b4667fba74b73a23302 Mon Sep 17 00:00:00 2001
From: Nathaniel Shead <nathanieloshead@gmail.com>
Date: Fri, 24 Nov 2023 16:26:43 +1100
Subject: [PATCH] c++: Follow module grammar more closely [PR110808]

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 37536faf2cf7..bc1683bef344 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14894,58 +14894,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
@@ -15003,9 +15009,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;
 
@@ -15017,7 +15026,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,
@@ -15045,7 +15057,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 000000000000..fadaba0b5608
--- /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 000000000000..09f956ff36f0
--- /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 000000000000..1ade029495ce
--- /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 000000000000..2351f28f9092
--- /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 78a53d2fda3a..db57adcef44d 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 78a53d2fda3a..db57adcef44d 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;
-- 
GitLab