diff mbox series

[c++] Disambiguate ModuleKind flags

Message ID bcc56e1d-46bc-a59f-a59f-31629a0b498f@acm.org
State New
Headers show
Series [c++] Disambiguate ModuleKind flags | expand

Commit Message

Nathan Sidwell May 10, 2022, 11:09 a.m. UTC
In modules, 'attached to global module' nearly always means 'not in
module purview'.  Also the implementation treats, 'in global module &&
in module purview' as meaning 'header unit'.  The ModuleKind flags
reflected that.  The 'nearly always' means there are cases that the
first condition is not invariant, and that of course invalidates the
second equivalence.

This disambiguates the ModuleKind flags to allow that 'not quite', and
separate out header-unitness from the GMF & purview flags combination.

1) Separate out named-module vs header-unit from the MODULE/GLOBAL flags.

2) Replace the MODULE/GLOBAL flags with PURVIEW & ATTACH flags.

3) Adjust the parser state handling.

Lays ground-work for language-declaration changes.

nathan
diff mbox series

Patch

From 92d6a0d28bff543bb2ccb9d9ccadb9ef3349ee29 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nathan@acm.org>
Date: Mon, 9 May 2022 04:40:43 -0700
Subject: [PATCH] [c++] Disambiguate ModuleKind flags

In modules, 'attached to global module' nearly always means 'not in
module purview'.  Also the implementation treats, 'in global module &&
in module purview' as meaning 'header unit'.  The ModuleKind flags
reflected that.  The 'nearly always' means there are cases that the
first condition is not invariant, and that of course invalidates the
second equivalence.

This disambiguates the ModuleKind flags to allow that 'not quite', and
separate out header-unitness from the GMF & purview flags combination.

1) Separate out named-module vs header-unit from the MODULE/GLOBAL flags.

2) Replace the MODULE/GLOBAL flags with PURVIEW & ATTACH flags.

3) Adjust the parser state handling.

Lays ground-work for language-declaration changes.

	gcc/cp/
	* cp-tree.h (enum module_kind_bits): Disambiguate purview,
	attach, named module vs header-unit.
	(global_purview_p, not_module_p): Delete.
	(named_module_p): New.
	(header_module_p, module_purview_p): Adjust.
	(module_attach_p, named_module_purview_p): New.
	* decl.cc (duplicate_decls): Adjust.
	* module.cc (declare_module, preprocessed_module): Adjust.
	* name-lookup.cc (init_global_partition): Adjust.
	(get_fixed_binding_slot, pushdecl): Adjust.
	* parser.cc (cp_parser_module_declaration): Adjust.
	(cp_parser_import_declaration, cp_parser_declaration): Adjust.
---
 gcc/cp/cp-tree.h      | 51 +++++++++++++++++++-----------------------
 gcc/cp/decl.cc        |  2 +-
 gcc/cp/module.cc      | 29 +++++++++++-------------
 gcc/cp/name-lookup.cc | 24 ++++++++++----------
 gcc/cp/parser.cc      | 52 ++++++++++++++++++++++---------------------
 5 files changed, 76 insertions(+), 82 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 9fb07d8ea39..8a5057a4dff 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1720,7 +1720,8 @@  check_constraint_info (tree t)
 #define DECL_MODULE_CHECK(NODE)						\
   TREE_NOT_CHECK (NODE, TEMPLATE_DECL)
 
-/* In the purview of a module (including header unit).  */
+/* In the purview of a named module (or in the purview of the
+   header-unit being compiled).  */
 #define DECL_MODULE_PURVIEW_P(N) \
   (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (N))->u.base.module_purview_p)
 
@@ -7137,40 +7138,26 @@  inline bool modules_p () { return flag_modules != 0; }
 /* The kind of module or part thereof that we're in.  */
 enum module_kind_bits
 {
-  MK_MODULE = 1 << 0,     /* This TU is a module.  */
-  MK_GLOBAL = 1 << 1,     /* Entities are in the global module.  */
-  MK_INTERFACE = 1 << 2,  /* This TU is an interface.  */
-  MK_PARTITION = 1 << 3,  /* This TU is a partition.  */
-  MK_EXPORTING = 1 << 4,  /* We are in an export region.  */
+  MK_NAMED = 1 << 0,	// TU is a named module
+  MK_HEADER = 1 << 1,	// TU is a header unit
+  MK_INTERFACE = 1 << 2,  // TU is an interface
+  MK_PARTITION = 1 << 3,  // TU is a partition
+
+  MK_PURVIEW = 1 << 4,	// In purview of current module
+  MK_ATTACH = 1 << 5,	// Attaching to named module
+
+  MK_EXPORTING = 1 << 6,  /* We are in an export region.  */
 };
 
 /* We do lots of bit-manipulation, so an unsigned is easier.  */
 extern unsigned module_kind;
 
-/*  MK_MODULE & MK_GLOBAL have the following combined meanings:
- MODULE GLOBAL
-   0	  0	not a module
-   0	  1	GMF of named module (we've not yet seen module-decl)
-   1	  0	purview of named module
-   1	  1	header unit.   */
-
-inline bool module_purview_p ()
-{ return module_kind & MK_MODULE; }
-inline bool global_purview_p ()
-{ return module_kind & MK_GLOBAL; }
-
-inline bool not_module_p ()
-{ return (module_kind & (MK_MODULE | MK_GLOBAL)) == 0; }
+inline bool module_p ()
+{ return module_kind & (MK_NAMED | MK_HEADER); }
 inline bool named_module_p ()
-{ /* This is a named module if exactly one of MODULE and GLOBAL is
-     set.  */
-  /* The divides are constant shifts!  */
-  return ((module_kind / MK_MODULE) ^ (module_kind / MK_GLOBAL)) & 1;
-}
+{ return module_kind & MK_NAMED; }
 inline bool header_module_p ()
-{ return (module_kind & (MK_MODULE | MK_GLOBAL)) == (MK_MODULE | MK_GLOBAL); }
-inline bool named_module_purview_p ()
-{ return (module_kind & (MK_MODULE | MK_GLOBAL)) == MK_MODULE; }
+{ return module_kind & MK_HEADER; }
 inline bool module_interface_p ()
 { return module_kind & MK_INTERFACE; }
 inline bool module_partition_p ()
@@ -7178,6 +7165,14 @@  inline bool module_partition_p ()
 inline bool module_has_cmi_p ()
 { return module_kind & (MK_INTERFACE | MK_PARTITION); }
 
+inline bool module_purview_p ()
+{ return module_kind & MK_PURVIEW; }
+inline bool module_attach_p ()
+{ return module_kind & MK_ATTACH; }
+
+inline bool named_module_purview_p ()
+{ return named_module_p () && module_purview_p (); }
+
 /* We're currently exporting declarations.  */
 inline bool module_exporting_p ()
 { return module_kind & MK_EXPORTING; }
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 9c9cf9f7f6b..4099fdeca5a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -2110,7 +2110,7 @@  duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
     {
       if (DECL_ARTIFICIAL (olddecl))
 	{
-	  if (!(global_purview_p () || not_module_p ()))
+	  if (module_attach_p ())
 	    error ("declaration %qD conflicts with builtin", newdecl);
 	  else
 	    DECL_MODULE_EXPORT_P (olddecl) = DECL_MODULE_EXPORT_P (newdecl);
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 6126316a6a0..bd4771bef72 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18971,25 +18971,21 @@  declare_module (module_state *module, location_t from_loc, bool exporting_p,
   gcc_checking_assert (module->is_direct () && module->has_location ());
 
   /* Yer a module, 'arry.  */
-  module_kind &= ~MK_GLOBAL;
-  module_kind |= MK_MODULE;
+  module_kind = module->is_header () ? MK_HEADER : MK_NAMED | MK_ATTACH;
 
-  if (module->is_partition () || exporting_p)
+  // Even in header units, we consider the decls to be purview
+  module_kind |= MK_PURVIEW;
+
+  if (module->is_partition ())
+    module_kind |= MK_PARTITION;
+  if (exporting_p)
     {
-      gcc_checking_assert (module->get_flatname ());
-
-      if (module->is_partition ())
-	module_kind |= MK_PARTITION;
-
-      if (exporting_p)
-	{
-	  module->interface_p = true;
-	  module_kind |= MK_INTERFACE;
-	}
-
-      if (module->is_header ())
-	module_kind |= MK_GLOBAL | MK_EXPORTING;
+      module->interface_p = true;
+      module_kind |= MK_INTERFACE;
+    }
 
+  if (module_has_cmi_p ())
+    {
       /* Copy the importing information we may have already done.  We
 	 do not need to separate out the imports that only happen in
 	 the GMF, inspite of what the literal wording of the std
@@ -19523,6 +19519,7 @@  preprocessed_module (cpp_reader *reader)
 	  if (module->is_module ())
 	    {
 	      declare_module (module, cpp_main_loc (reader), true, NULL, reader);
+	      module_kind |= MK_EXPORTING;
 	      break;
 	    }
 	}
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 7b0638d3166..a05244df74e 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -190,25 +190,25 @@  search_imported_binding_slot (tree *slot, unsigned ix)
 static void
 init_global_partition (binding_cluster *cluster, tree decl)
 {
-  bool purview = true;
+  bool named = true;
 
   if (header_module_p ())
-    purview = false;
+    named = false;
   else if (TREE_PUBLIC (decl)
 	   && TREE_CODE (decl) == NAMESPACE_DECL
 	   && !DECL_NAMESPACE_ALIAS (decl))
-    purview = false;
+    named = false;
   else if (!get_originating_module (decl))
-    purview = false;
+    named = false;
 
   binding_slot *mslot;
-  if (!purview)
-    mslot = &cluster[0].slots[BINDING_SLOT_GLOBAL];
-  else
+  if (named)
     mslot = &cluster[BINDING_SLOT_PARTITION
 		     / BINDING_VECTOR_SLOTS_PER_CLUSTER]
       .slots[BINDING_SLOT_PARTITION
 	     % BINDING_VECTOR_SLOTS_PER_CLUSTER];
+  else
+    mslot = &cluster[0].slots[BINDING_SLOT_GLOBAL];
 
   if (*mslot)
     decl = ovl_make (decl, *mslot);
@@ -248,7 +248,7 @@  get_fixed_binding_slot (tree *slot, tree name, unsigned ix, int create)
       if (!create)
 	return NULL;
 
-      /* The partition slot is only needed when we know we're a named
+      /* The partition slot is only needed when we're a named
 	 module.  */
       bool partition_slot = named_module_p ();
       unsigned want = ((BINDING_SLOTS_FIXED + partition_slot + (create < 0)
@@ -3472,9 +3472,9 @@  push_local_extern_decl_alias (tree decl)
   DECL_LOCAL_DECL_ALIAS (decl) = alias;
 }
 
-/* DECL is a global or module-purview entity.  If it has non-internal
-   linkage, and we have a module vector, record it in the appropriate
-   slot.  We have already checked for duplicates.  */
+/* If DECL has non-internal linkage, and we have a module vector,
+   record it in the appropriate slot.  We have already checked for
+   duplicates.  */
 
 static void
 maybe_record_mergeable_decl (tree *slot, tree name, tree decl)
@@ -3826,7 +3826,7 @@  pushdecl (tree decl, bool hiding)
 
 	  if (level->kind == sk_namespace
 	      && TREE_PUBLIC (level->this_entity)
-	      && !not_module_p ())
+	      && module_p ())
 	    maybe_record_mergeable_decl (slot, name, decl);
 	}
     }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 84b45cf47ec..8da02de95fb 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14582,7 +14582,7 @@  cp_parser_module_declaration (cp_parser *parser, module_parse mp_state,
     {
       /* Start global module fragment.  */
       cp_lexer_consume_token (parser->lexer);
-      module_kind |= MK_GLOBAL;
+      module_kind = MK_NAMED;
       mp_state = MP_GLOBAL;
       cp_parser_require_pragma_eol (parser, token);
     }
@@ -14596,16 +14596,16 @@  cp_parser_module_declaration (cp_parser *parser, module_parse mp_state,
       cp_lexer_consume_token (parser->lexer);
       cp_parser_require_pragma_eol (parser, token);
 
-      if (!(mp_state == MP_PURVIEW || mp_state == MP_PURVIEW_IMPORTS)
-	  || !module_interface_p () || module_partition_p ())
-	error_at (token->location,
-		  "private module fragment only permitted in purview"
-		  " of module interface or partition");
-      else
+      if ((mp_state == MP_PURVIEW || mp_state == MP_PURVIEW_IMPORTS)
+	  && module_has_cmi_p ())
 	{
 	  mp_state = MP_PRIVATE_IMPORTS;
 	  sorry_at (token->location, "private module fragment");
 	}
+      else
+	error_at (token->location,
+		  "private module fragment only permitted in purview"
+		  " of module interface or partition");
     }
   else if (!(mp_state == MP_FIRST || mp_state == MP_GLOBAL))
     {
@@ -14642,10 +14642,7 @@  cp_parser_import_declaration (cp_parser *parser, module_parse mp_state,
   parser->lexer->in_pragma = true;
   cp_token *token = cp_lexer_consume_token (parser->lexer);
 
-  if (mp_state != MP_PURVIEW_IMPORTS
-      && mp_state != MP_PRIVATE_IMPORTS
-      && module_purview_p ()
-      && !global_purview_p ())
+  if (mp_state == MP_PURVIEW || mp_state == MP_PRIVATE)
     {
       error_at (token->location, "post-module-declaration"
 		" imports must be contiguous");
@@ -14674,18 +14671,19 @@  cp_parser_import_declaration (cp_parser *parser, module_parse mp_state,
 	error_at (token->location, "import cannot appear directly in"
 		  " a linkage-specification");
 
-      /* Module-purview imports must not be from source inclusion
-	 [cpp.import]/7  */
-      if (attrs && module_purview_p () && !global_purview_p ()
-	  && private_lookup_attribute ("__translated",
-				       strlen ("__translated"), attrs))
-	error_at (token->location, "post-module-declaration imports"
-		  " must not be include-translated");
-      else if ((mp_state == MP_PURVIEW_IMPORTS
-		|| mp_state == MP_PRIVATE_IMPORTS)
-	       && !token->main_source_p)
-	error_at (token->location, "post-module-declaration imports"
-		  " must not be from header inclusion");
+      if (mp_state == MP_PURVIEW_IMPORTS || mp_state == MP_PRIVATE_IMPORTS)
+	{
+	  /* Module-purview imports must not be from source inclusion
+	     [cpp.import]/7  */
+	  if (attrs
+	      && private_lookup_attribute ("__translated",
+					   strlen ("__translated"), attrs))
+	    error_at (token->location, "post-module-declaration imports"
+		      " must not be include-translated");
+	  else if (!token->main_source_p)
+	    error_at (token->location, "post-module-declaration imports"
+		      " must not be from header inclusion");
+	}
 
       import_module (mod, token->location, exporting, attrs, parse_in);
     }
@@ -14941,10 +14939,14 @@  cp_parser_declaration (cp_parser* parser, tree prefix_attrs)
       cp_token *next = exporting ? token2 : token1;
       if (exporting)
 	cp_lexer_consume_token (parser->lexer);
+      // In module purview this will be ill-formed.
+      auto state = (!named_module_p () ? MP_NOT_MODULE
+		    : module_purview_p () ? MP_PURVIEW
+		    : MP_GLOBAL);
       if (next->keyword == RID__MODULE)
-	cp_parser_module_declaration (parser, MP_NOT_MODULE, exporting);
+	cp_parser_module_declaration (parser, state, exporting);
       else
-	cp_parser_import_declaration (parser, MP_NOT_MODULE, exporting);
+	cp_parser_import_declaration (parser, state, exporting);
     }
   /* If the next token is `extern', 'static' or 'inline' and the one
      after that is `template', we have a GNU extended explicit
-- 
2.30.2