diff mbox series

[v3,4/5] c++/modules: Check linkage for exported declarations

Message ID 670928bb.170a0220.48198.aa1a@mx.google.com
State New
Headers show
Series c++/modules: Implement P1815 "Translation-unit-local entities" | expand

Commit Message

Nathaniel Shead Oct. 11, 2024, 1:31 p.m. UTC 
Updated to use 'unnamed namespace' instead, added some more details to
the internal linkage namespace diagnostic.

-- >8 --

By [module.interface] p3, if an exported declaration is not within a
header unit, it shall not declare a name with internal linkage.

Unfortunately we cannot just do this within set_originating_module,
since at the locations its called the linkage for declarations are not
always fully determined yet.  We could move the calls but this causes
the checking assertion to fail as the originating module declaration may
have moved, and in general for some kinds of declarations it's not
always obvious where it should be moved to.

This patch instead introduces a new function to check that the linkage
of a declaration within a module is correct, to be called for all
declarations once their linkage is fully determined.

As a drive-by fix this patch also improves the source location of
namespace aliases to point at the identifier rather than the terminating
semicolon.

gcc/cp/ChangeLog:

	* cp-tree.h (check_module_decl_linkage): Declare.
	* decl2.cc (finish_static_data_member_decl): Check linkage.
	* module.cc (set_originating_module): Adjust comment.
	(check_module_decl_linkage): New function.
	* name-lookup.cc (do_namespace_alias): Build alias with
	specified location, check linkage.
	(pushtag): Check linkage.
	(push_namespace): Slightly clarify error message.
	* name-lookup.h (do_namespace_alias): Add location parameter.
	* parser.cc (cp_parser_namespace_alias_definition): Pass
	identifier location to do_namespace_alias.
	(cp_parser_alias_declaration): Check linkage.
	(cp_parser_init_declarator): Check linkage.
	(cp_parser_function_definition_after_declarator): Check linkage.
	(cp_parser_save_member_function_body): Check linkage.
	* pt.cc (finish_concept_definition): Mark as public, check
	linkage.

libcc1/ChangeLog:

	* libcp1plugin.cc (plugin_add_namespace_alias): Call
	do_namespace_alias with input_location.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/export-3.C: Adjust error message.
	* g++.dg/modules/export-6.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>
---
 gcc/cp/cp-tree.h                        |  1 +
 gcc/cp/decl2.cc                         |  1 +
 gcc/cp/module.cc                        | 29 +++++++++++++++++---
 gcc/cp/name-lookup.cc                   | 20 +++++++++++---
 gcc/cp/name-lookup.h                    |  2 +-
 gcc/cp/parser.cc                        |  9 ++++++-
 gcc/cp/pt.cc                            |  2 ++
 gcc/testsuite/g++.dg/modules/export-3.C |  2 +-
 gcc/testsuite/g++.dg/modules/export-6.C | 35 +++++++++++++++++++++++++
 libcc1/libcp1plugin.cc                  |  2 +-
 10 files changed, 92 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/export-6.C
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a71d0704f8b..f57a9b0b6ce 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7456,6 +7456,7 @@  extern void set_originating_module (tree, bool friend_p = false);
 extern tree get_originating_module_decl (tree) ATTRIBUTE_PURE;
 extern int get_originating_module (tree, bool for_mangle = false) ATTRIBUTE_PURE;
 extern unsigned get_importing_module (tree, bool = false) ATTRIBUTE_PURE;
+extern void check_module_decl_linkage (tree);
 
 /* Where current instance of the decl got declared/defined/instantiated.  */
 extern void set_instantiating_module (tree);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 0279372488c..97ce4473b1c 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -1019,6 +1019,7 @@  finish_static_data_member_decl (tree decl,
     }
 
   cp_finish_decl (decl, init, init_const_expr_p, asmspec_tree, flags);
+  check_module_decl_linkage (decl);
 }
 
 /* DECLARATOR and DECLSPECS correspond to a class member.  The other
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f75e211e0e1..456b73d59b6 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19926,11 +19926,34 @@  set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED)
       DECL_MODULE_ATTACH_P (decl) = true;
     }
 
-  if (!module_exporting_p ())
+  /* It is ill-formed to export a declaration with internal linkage.  However,
+     at the point this function is called we don't yet always know whether this
+     declaration has internal linkage; instead we defer this check for callers
+     to do once visibility has been determined.  */
+  if (module_exporting_p ())
+    DECL_MODULE_EXPORT_P (decl) = true;
+}
+
+/* Checks whether DECL within a module unit has valid linkage for its kind.
+   Must be called after visibility for DECL has been finalised.  */
+
+void
+check_module_decl_linkage (tree decl)
+{
+  if (!module_has_cmi_p ())
     return;
 
-  // FIXME: Check ill-formed linkage
-  DECL_MODULE_EXPORT_P (decl) = true;
+  /* An internal-linkage declaration cannot be generally be exported.
+     But it's OK to export any declaration from a header unit, including
+     internal linkage declarations.  */
+  if (!header_module_p ()
+      && DECL_MODULE_EXPORT_P (decl)
+      && decl_linkage (decl) == lk_internal)
+    {
+      error_at (DECL_SOURCE_LOCATION (decl),
+		"exporting declaration %qD with internal linkage", decl);
+      DECL_MODULE_EXPORT_P (decl) = false;
+    }
 }
 
 /* DECL is keyed to CTX for odr purposes.  */
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 22391110fd7..10d8f4bd87b 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -6605,7 +6605,7 @@  pop_decl_namespace (void)
 /* Process a namespace-alias declaration.  */
 
 void
-do_namespace_alias (tree alias, tree name_space)
+do_namespace_alias (location_t loc, tree alias, tree name_space)
 {
   if (name_space == error_mark_node)
     return;
@@ -6615,12 +6615,13 @@  do_namespace_alias (tree alias, tree name_space)
   name_space = ORIGINAL_NAMESPACE (name_space);
 
   /* Build the alias.  */
-  alias = build_lang_decl (NAMESPACE_DECL, alias, void_type_node);
+  alias = build_lang_decl_loc (loc, NAMESPACE_DECL, alias, void_type_node);
   DECL_NAMESPACE_ALIAS (alias) = name_space;
   DECL_EXTERNAL (alias) = 1;
   DECL_CONTEXT (alias) = FROB_CONTEXT (current_scope ());
   TREE_PUBLIC (alias) = TREE_PUBLIC (DECL_CONTEXT (alias));
   set_originating_module (alias);
+  check_module_decl_linkage (alias);
 
   pushdecl (alias);
 
@@ -8550,6 +8551,7 @@  pushtag (tree name, tree type, TAG_how how)
   /* Set type visibility now if this is a forward declaration.  */
   TREE_PUBLIC (decl) = 1;
   determine_visibility (decl);
+  check_module_decl_linkage (decl);
 
   return type;
 }
@@ -9255,8 +9257,18 @@  push_namespace (tree name, bool make_inline)
 	  if (TREE_PUBLIC (ns))
 	    DECL_MODULE_EXPORT_P (ns) = true;
 	  else if (!header_module_p ())
-	    error_at (input_location,
-		      "exporting namespace with internal linkage");
+	    {
+	      if (name)
+		{
+		  auto_diagnostic_group d;
+		  error_at (input_location, "exporting namespace %qD with "
+			    "internal linkage", ns);
+		  inform (input_location, "%qD has internal linkage because "
+			  "it was declared in an unnamed namespace", ns);
+		}
+	      else
+		error_at (input_location, "exporting unnamed namespace");
+	    }
 	}
       if (module_purview_p ())
 	DECL_MODULE_PURVIEW_P (ns) = true;
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index f39f5309150..26b3b1ffc62 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -444,7 +444,7 @@  extern tree cp_namespace_decls (tree);
 extern void set_decl_namespace (tree, tree, bool);
 extern void push_decl_namespace (tree);
 extern void pop_decl_namespace (void);
-extern void do_namespace_alias (tree, tree);
+extern void do_namespace_alias (location_t, tree, tree);
 extern tree do_class_using_decl (tree, tree);
 extern tree lookup_arg_dependent (tree, tree, vec<tree, va_gc> *);
 extern tree search_anon_aggr (tree, tree, bool = false);
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 9d31a975dcf..393d7b91aee 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -22396,6 +22396,7 @@  cp_parser_namespace_alias_definition (cp_parser* parser)
   /* Look for the `namespace' keyword.  */
   cp_parser_require_keyword (parser, RID_NAMESPACE, RT_NAMESPACE);
   /* Look for the identifier.  */
+  location_t id_location = cp_lexer_peek_token (parser->lexer)->location;
   identifier = cp_parser_identifier (parser);
   if (identifier == error_mark_node)
     return;
@@ -22419,7 +22420,7 @@  cp_parser_namespace_alias_definition (cp_parser* parser)
   cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
 
   /* Register the alias in the symbol table.  */
-  do_namespace_alias (identifier, namespace_specifier);
+  do_namespace_alias (id_location, identifier, namespace_specifier);
 }
 
 /* Parse a qualified-namespace-specifier.
@@ -22868,6 +22869,8 @@  cp_parser_alias_declaration (cp_parser* parser)
 	check_member_template (decl);
     }
 
+  check_module_decl_linkage (decl);
+
   return decl;
 }
 
@@ -23925,6 +23928,7 @@  cp_parser_init_declarator (cp_parser* parser,
 			 `explicit' constructor cannot be used.  */
 		      ((is_direct_init || !is_initialized)
 		       ? LOOKUP_NORMAL : LOOKUP_IMPLICIT));
+      check_module_decl_linkage (decl);
     }
   else if ((cxx_dialect != cxx98) && friend_p
 	   && decl && TREE_CODE (decl) == FUNCTION_DECL)
@@ -33161,6 +33165,7 @@  cp_parser_function_definition_after_declarator (cp_parser* parser,
 
   /* Finish the function.  */
   fn = finish_function (inline_p);
+  check_module_decl_linkage (fn);
 
   if (modules_p ()
       && !inline_p
@@ -33815,6 +33820,8 @@  cp_parser_save_member_function_body (cp_parser* parser,
   /* Add FN to the queue of functions to be parsed later.  */
   vec_safe_push (unparsed_funs_with_definitions, fn);
 
+  check_module_decl_linkage (fn);
+
   return fn;
 }
 
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index c7c6da0969e..81d0a8f5203 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -29733,11 +29733,13 @@  finish_concept_definition (cp_expr id, tree init, tree attrs)
   tree decl = build_lang_decl_loc (loc, CONCEPT_DECL, *id, boolean_type_node);
   DECL_CONTEXT (decl) = current_scope ();
   DECL_INITIAL (decl) = init;
+  TREE_PUBLIC (decl) = true;
 
   if (attrs)
     cplus_decl_attributes (&decl, attrs, 0);
 
   set_originating_module (decl, false);
+  check_module_decl_linkage (decl);
 
   /* Push the enclosing template.  */
   return push_template_decl (decl);
diff --git a/gcc/testsuite/g++.dg/modules/export-3.C b/gcc/testsuite/g++.dg/modules/export-3.C
index 6af314b9519..5a001d7cff2 100644
--- a/gcc/testsuite/g++.dg/modules/export-3.C
+++ b/gcc/testsuite/g++.dg/modules/export-3.C
@@ -25,4 +25,4 @@  namespace {
   export namespace ns {}  // { dg-error "internal linkage" }
 }
 
-export namespace {}  // { dg-error "internal linkage" }
+export namespace {}  // { dg-error "exporting unnamed namespace" }
diff --git a/gcc/testsuite/g++.dg/modules/export-6.C b/gcc/testsuite/g++.dg/modules/export-6.C
new file mode 100644
index 00000000000..3dd3876b13e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/export-6.C
@@ -0,0 +1,35 @@ 
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi !bad }
+
+export module bad;
+
+export static int x = 123;  // { dg-error "internal linkage" }
+export static void f();  // { dg-error "internal linkage" }
+export static void g() {}  // { dg-error "internal linkage" }
+export template <typename T> static void t();  // { dg-error "internal linkage" }
+export template <typename T> static void u() {}  // { dg-error "internal linkage" }
+
+namespace {
+  export int y = 456;  // { dg-error "internal linkage" }
+  export void h();  // { dg-error "internal linkage" }
+  export void i() {}  // { dg-error "internal linkage" }
+  export template <typename T> void v(); // { dg-error "internal linkage" }
+  export template <typename T> void w() {} // { dg-error "internal linkage" }
+
+  export namespace ns {}  // { dg-error "internal linkage" }
+
+  export struct A {};  // { dg-error "internal linkage" }
+  export template <typename T> struct B {};  // { dg-error "internal linkage" }
+
+  export enum E {};  // { dg-error "internal linkage" }
+  export enum class F {};  // { dg-error "internal linkage" }
+
+  export template <typename T> using U = int;  // { dg-error "internal linkage" }
+
+#if __cplusplus >= 202002L
+  export template <typename T> concept C = true;  // { dg-error "internal linkage" "" { target c++20 } }
+#endif
+}
+
+export namespace {}  // { dg-error "exporting unnamed namespace" }
+export namespace ns2 = ns;  // { dg-error "internal linkage" }
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index da68c5d0ac1..97877ad9138 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -799,7 +799,7 @@  plugin_add_namespace_alias (cc1_plugin::connection *,
   tree name = get_identifier (id);
   tree target = convert_in (target_in);
 
-  do_namespace_alias (name, target);
+  do_namespace_alias (input_location, name, target);
 
   return 1;
 }