On 9/27/24 1:59 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu and aarch64-unknown-linux-gnu,
> OK for trunk?
>
> -- >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.
>
> @@ -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 illegal to export a declaration with internal linkage. However, at
s/illegal/ill-formed/
> @@ -9249,8 +9251,13 @@ 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)
> + error_at (input_location,
> + "exporting namespace %qD with internal linkage", ns);
Since a namespace can only have internal linkage if it's within an
unnamed namespace, maybe mention that?
> + else
> + error_at (input_location, "exporting anonymous namespace");
Let's move away from using the word "anonymous" for unnamed namespaces.
Jason
@@ -7455,6 +7455,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);
@@ -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
@@ -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 illegal to export a declaration with internal linkage. However, at
+ the point this function is called we don't always know yet 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. */
@@ -6598,7 +6598,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;
@@ -6608,12 +6608,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);
@@ -8546,6 +8547,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;
}
@@ -9249,8 +9251,13 @@ 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)
+ error_at (input_location,
+ "exporting namespace %qD with internal linkage", ns);
+ else
+ error_at (input_location, "exporting anonymous namespace");
+ }
}
if (module_purview_p ())
DECL_MODULE_PURVIEW_P (ns) = true;
@@ -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);
@@ -22365,6 +22365,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;
@@ -22388,7 +22389,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.
@@ -22837,6 +22838,8 @@ cp_parser_alias_declaration (cp_parser* parser)
check_member_template (decl);
}
+ check_module_decl_linkage (decl);
+
return decl;
}
@@ -23894,6 +23897,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)
@@ -33140,6 +33144,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
@@ -33794,6 +33799,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;
}
@@ -29724,11 +29724,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);
@@ -25,4 +25,4 @@ namespace {
export namespace ns {} // { dg-error "internal linkage" }
}
-export namespace {} // { dg-error "internal linkage" }
+export namespace {} // { dg-error "exporting anonymous namespace" }
new file mode 100644
@@ -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 anonymous namespace" }
+export namespace ns2 = ns; // { dg-error "internal linkage" }
@@ -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;
}