[PATCH v3 0/5] c++/modules: Implement P1815 "Translation-unit-local entities"
Nathaniel Shead
nathanieloshead@gmail.com
Thu Oct 31 10:12:43 GMT 2024
Gentle ping for the first two patches in this series:
https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665108.html
On Sat, Oct 12, 2024 at 12:29:09AM +1100, Nathaniel Shead wrote:
> This patch series implements most of the changes made by P1815. It also
> cleans up a few bugs found along the way that impacted tests I wrote.
>
> The whole patch series was bootstrapped on x86_64-pc-linux-gnu and
> aarch64-unknown-linux-gnu with no regressions.
>
> A range-diff against v2 is attached; the main changes are responses to
> comments left on previous versions of this patch series.
>
> Nathaniel Shead (5):
> c++/modules: Detect exposures of TU-local entities
> c++/modules: Ignore TU-local entities where necessary
> c++/modules: Support unnamed namespaces in header units
> c++/modules: Check linkage for exported declarations
> c++/modules: Validate external linkage definitions in header units
> [PR116401]
>
> gcc/c-family/c.opt | 4 +
> gcc/cp/cp-objcp-common.cc | 1 +
> gcc/cp/cp-tree.def | 6 +
> gcc/cp/cp-tree.h | 22 +-
> gcc/cp/decl.cc | 4 +
> gcc/cp/decl2.cc | 1 +
> gcc/cp/module.cc | 749 +++++++++++++++---
> gcc/cp/name-lookup.cc | 30 +-
> gcc/cp/name-lookup.h | 2 +-
> gcc/cp/parser.cc | 9 +-
> gcc/cp/pt.cc | 100 ++-
> gcc/doc/invoke.texi | 19 +-
> gcc/testsuite/g++.dg/modules/block-decl-2.C | 2 +-
> gcc/testsuite/g++.dg/modules/export-3.C | 2 +-
> gcc/testsuite/g++.dg/modules/export-6.C | 35 +
> gcc/testsuite/g++.dg/modules/hdr-2.H | 172 ++++
> gcc/testsuite/g++.dg/modules/internal-1.C | 15 +-
> gcc/testsuite/g++.dg/modules/internal-3.C | 18 +
> gcc/testsuite/g++.dg/modules/internal-4_a.H | 4 +
> gcc/testsuite/g++.dg/modules/internal-4_b.C | 124 +++
> gcc/testsuite/g++.dg/modules/internal-5_a.C | 110 +++
> gcc/testsuite/g++.dg/modules/internal-5_b.C | 30 +
> gcc/testsuite/g++.dg/modules/internal-6.C | 24 +
> gcc/testsuite/g++.dg/modules/internal-7_a.C | 75 ++
> gcc/testsuite/g++.dg/modules/internal-7_b.C | 21 +
> gcc/testsuite/g++.dg/modules/internal-8_a.C | 35 +
> gcc/testsuite/g++.dg/modules/internal-9_a.H | 28 +
> gcc/testsuite/g++.dg/modules/internal-9_b.C | 29 +
> gcc/testsuite/g++.dg/modules/linkage-2.C | 5 +-
> gcc/testsuite/g++.dg/modules/macro-4_c.H | 2 +-
> gcc/testsuite/g++.dg/modules/pr106761.h | 2 +-
> gcc/testsuite/g++.dg/modules/pr98843_b.H | 2 +-
> gcc/testsuite/g++.dg/modules/pr99468.H | 2 +-
> gcc/testsuite/g++.dg/modules/pragma-1_a.H | 2 +-
> gcc/testsuite/g++.dg/modules/tpl-ary-1.h | 2 +-
> .../g++.dg/modules/xtreme-header-8.C | 8 +
> libcc1/libcp1plugin.cc | 2 +-
> 37 files changed, 1555 insertions(+), 143 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/export-6.C
> create mode 100644 gcc/testsuite/g++.dg/modules/hdr-2.H
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-3.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-4_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-4_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-5_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-5_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-6.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-7_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-7_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-8_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-9_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/internal-9_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/xtreme-header-8.C
>
> Range-diff against v2:
> 1: 4a032de3f30 ! 1: 36b944bfe8c c++/modules: Detect exposures of TU-local entities
> @@ Commit message
> follow-up patch for easier review, as it has broader implications for
> streaming.
>
> + TU-local lambdas are also not yet properly implemented, due to other
> + bugs with regards to LAMBDA_TYPE_EXTRA_SCOPE not being set in all cases
> + that it probably should be (see also PR c++/116568). We can revisit
> + this once that issue has been fixed.
> +
> Finally, this patch makes a couple of small adjustments to the modules
> streaming logic to prune any leftover TU-local deps (that aren't
> erroneous exposures). This is required for this patch to ensure that
> @@ Commit message
>
> gcc/cp/ChangeLog:
>
> - * cp-tree.h (TYPE_DEPENDENT_P_VALID): Fix whitespace.
> - (TYPE_DEFINED_IN_INITIALIZER_P): New accessor.
> * module.cc (DB_IS_INTERNAL_BIT): Rename to...
> (DB_TU_LOCAL_BIT): ...this.
> (DB_REFS_INTERNAL_BIT): Rename to...
> @@ Commit message
> diagnostic messages to report exposures of TU-local entities.
> (depset::tarjan::connect): Don't include any TU-local depsets.
> (depset::hash::connect): Likewise.
> - (trees_out::core_bools): Stream TYPE_LANG_FLAG_7.
> - (trees_in::core_bools): Read it.
> - * parser.h (struct cp_parser::in_initializer_p): New flag.
> - * parser.cc (cp_debug_parser): Print the new flag.
> - (cp_parser_new): Set the new flag to false.
> - (cp_parser_lambda_expression): Mark whether the lambda was
> - defined in an initializer.
> - (cp_parser_initializer): Set the new flag to true while parsing.
> - (cp_parser_class_head): Mark whether the class was defined in an
> - initializer.
> - (cp_parser_concept_definition): Set the new flag to true while
> - parsing.
>
> gcc/testsuite/ChangeLog:
>
> @@ Commit message
> * g++.dg/modules/linkage-2.C: Adjust messages, remove XFAILS.
> * g++.dg/modules/internal-3.C: New test.
> * g++.dg/modules/internal-4_a.H: New test.
> - * g++.dg/modules/internal-4_a.C: New test.
> + * g++.dg/modules/internal-4_b.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> -
> - ## gcc/cp/cp-tree.h ##
> -@@ gcc/cp/cp-tree.h: extern GTY(()) tree cp_global_trees[CPTI_MAX];
> - AUTO_IS_DECLTYPE (in TEMPLATE_TYPE_PARM)
> - TEMPLATE_TEMPLATE_PARM_SIMPLE_P (in TEMPLATE_TEMPLATE_PARM)
> - 6: TYPE_DEPENDENT_P_VALID
> -+ 7: TYPE_DEFINED_IN_INITIALIZER_P
> -
> - Usage of DECL_LANG_FLAG_?:
> - 0: DECL_TEMPLATE_PARM_P (in PARM_DECL, CONST_DECL, TYPE_DECL, or TEMPLATE_DECL)
> -@@ gcc/cp/cp-tree.h: enum languages { lang_c, lang_cplusplus };
> -
> - /* True if dependent_type_p has been called for this type, with the
> - result that TYPE_DEPENDENT_P is valid. */
> --#define TYPE_DEPENDENT_P_VALID(NODE) TYPE_LANG_FLAG_6(NODE)
> -+#define TYPE_DEPENDENT_P_VALID(NODE) TYPE_LANG_FLAG_6 (NODE)
> -+
> -+/* True if this type was defined in an initializer. Used for determining
> -+ whether an entity is TU-local. */
> -+#define TYPE_DEFINED_IN_INITIALIZER_P(NODE) TYPE_LANG_FLAG_7 (NODE)
> -
> - /* Nonzero if this type is const-qualified. */
> - #define CP_TYPE_CONST_P(NODE) \
> + Reviewed-by: Jason Merrill <jason@redhat.com>
>
> ## gcc/cp/module.cc ##
> @@ gcc/cp/module.cc: private:
> @@ gcc/cp/module.cc: public:
> private:
> static bool add_binding_entity (tree, WMB_Flags, void *);
>
> -@@ gcc/cp/module.cc: trees_out::core_bools (tree t, bits_out& bits)
> - WB (t->type_common.lang_flag_4);
> - WB (t->type_common.lang_flag_5);
> - WB (t->type_common.lang_flag_6);
> -+ WB (t->type_common.lang_flag_7);
> - WB (t->type_common.typeless_storage);
> - }
> -
> -@@ gcc/cp/module.cc: trees_in::core_bools (tree t, bits_in& bits)
> - RB (t->type_common.lang_flag_4);
> - RB (t->type_common.lang_flag_5);
> - RB (t->type_common.lang_flag_6);
> -+ RB (t->type_common.lang_flag_7);
> - RB (t->type_common.typeless_storage);
> - }
> -
> @@ gcc/cp/module.cc: depset::hash::find_binding (tree ctx, tree name)
> return slot ? *slot : NULL;
> }
> @@ gcc/cp/module.cc: depset::hash::find_binding (tree ctx, tree name)
> + is used to declare only TU-local entities.
> +
> + We consider types with names for linkage purposes as having names, since
> -+ these aren't really TU-local, and also consider constraint-expressions
> -+ as initializers. */
> ++ these aren't really TU-local. */
> + if (TREE_CODE (decl) == TYPE_DECL
> + && TYPE_ANON_P (type)
> + && !DECL_SELF_REFERENCE_P (decl)
> @@ gcc/cp/module.cc: depset::hash::find_binding (tree ctx, tree name)
> + && !(UNSCOPED_ENUM_P (type) && TYPE_VALUES (type)))
> + {
> + tree main_decl = TYPE_MAIN_DECL (type);
> -+ if (!TYPE_DEFINED_IN_INITIALIZER_P (type)
> -+ && !DECL_CLASS_SCOPE_P (main_decl)
> -+ && !decl_function_context (main_decl))
> ++ if (!DECL_CLASS_SCOPE_P (main_decl)
> ++ && !decl_function_context (main_decl)
> ++ /* FIXME: Lambdas defined outside initializers. We'll need to more
> ++ thoroughly set LAMBDA_TYPE_EXTRA_SCOPE to check this. */
> ++ && !LAMBDA_TYPE_P (type))
> + {
> + if (explain)
> + inform (loc, "%qT has no name and is not defined within a class, "
> @@ gcc/cp/module.cc: module_state::write_begin (elf_out *to, cpp_reader *reader,
> #endif
>
> - /* Determine Strongy Connected Components. */
> -+ /* Determine Strongy Connected Components. This will also strip any
> ++ /* Determine Strongly Connected Components. This will also strip any
> + unnecessary dependencies on imported or TU-local entities. */
> vec<depset *> sccs = table.connect ();
>
> vec_alloc (ool, modules->length ());
>
> - ## gcc/cp/parser.cc ##
> -@@ gcc/cp/parser.cc: cp_debug_parser (FILE *file, cp_parser *parser)
> - parser->in_unbraced_export_declaration_p);
> - cp_debug_print_flag (file, "Parsing a declarator",
> - parser->in_declarator_p);
> -+ cp_debug_print_flag (file, "Parsing an initializer",
> -+ parser->in_initializer_p);
> - cp_debug_print_flag (file, "In template argument list",
> - parser->in_template_argument_list_p);
> - cp_debug_print_flag (file, "Parsing an iteration statement",
> -@@ gcc/cp/parser.cc: cp_parser_new (cp_lexer *lexer)
> - /* We are not processing a declarator. */
> - parser->in_declarator_p = false;
> -
> -+ /* We are not processing an initializer. */
> -+ parser->in_initializer_p = false;
> -+
> - /* We are not processing a template-argument-list. */
> - parser->in_template_argument_list_p = false;
> -
> -@@ gcc/cp/parser.cc: cp_parser_lambda_expression (cp_parser* parser)
> -
> - record_lambda_scope (lambda_expr);
> - record_lambda_scope_discriminator (lambda_expr);
> -+ TYPE_DEFINED_IN_INITIALIZER_P (type) = parser->in_initializer_p;
> -
> - /* Do this again now that LAMBDA_EXPR_EXTRA_SCOPE is set. */
> - determine_visibility (TYPE_NAME (type));
> -@@ gcc/cp/parser.cc: cp_parser_initializer (cp_parser *parser, bool *is_direct_init /*=nullptr*/,
> - if (non_constant_p)
> - *non_constant_p = false;
> -
> -+ bool saved_in_initializer_p = parser->in_initializer_p;
> -+ parser->in_initializer_p = true;
> -+
> - if (token->type == CPP_EQ)
> - {
> - /* Consume the `='. */
> -@@ gcc/cp/parser.cc: cp_parser_initializer (cp_parser *parser, bool *is_direct_init /*=nullptr*/,
> - if (!subexpression_p && check_for_bare_parameter_packs (init))
> - init = error_mark_node;
> -
> -+ parser->in_initializer_p = saved_in_initializer_p;
> -+
> - return init;
> - }
> -
> -@@ gcc/cp/parser.cc: cp_parser_class_head (cp_parser* parser,
> - }
> - else if (type == error_mark_node)
> - type = NULL_TREE;
> -+ else
> -+ TYPE_DEFINED_IN_INITIALIZER_P (type) = parser->in_initializer_p;
> -
> - if (type)
> - {
> -@@ gcc/cp/parser.cc: cp_parser_concept_definition (cp_parser *parser)
> - }
> -
> - processing_constraint_expression_sentinel parsing_constraint;
> -+ parser->in_initializer_p = true;
> -+
> - tree init = cp_parser_constraint_expression (parser);
> - if (init == error_mark_node)
> - cp_parser_skip_to_end_of_statement (parser);
> -@@ gcc/cp/parser.cc: cp_parser_concept_definition (cp_parser *parser)
> - but continue as if it were. */
> - cp_parser_consume_semicolon_at_end_of_statement (parser);
> -
> -+ parser->in_initializer_p = false;
> - return finish_concept_definition (id, init, attrs);
> - }
> -
> -
> - ## gcc/cp/parser.h ##
> -@@ gcc/cp/parser.h: struct GTY(()) cp_parser {
> - direct-declarator. */
> - bool in_declarator_p;
> -
> -+ /* TRUE if we are parsing an initializer. */
> -+ bool in_initializer_p;
> -+
> - /* TRUE if we are presently parsing a template-argument-list. */
> - bool in_template_argument_list_p;
> -
> -
> ## gcc/testsuite/g++.dg/modules/block-decl-2.C ##
> @@ gcc/testsuite/g++.dg/modules/block-decl-2.C: export extern "C++" auto foo() {
> struct X {
> @@ gcc/testsuite/g++.dg/modules/internal-4_b.C (new)
> +auto in_initializer = []{}; // OK
> +
> +#if __cplusplus >= 202002L
> -+decltype([]{}) d_lambda; // { dg-error "exposes TU-local entity" "" { target c++20 } }
> ++decltype([]{}) d_lambda; // { dg-error "exposes TU-local entity" "" { xfail *-*-* } }
> +
> +template <typename T>
> +concept in_constraint_expression = requires {
> + // Strictly by the standard this is currently ill-formed
> + // (this is a constraint-expression not an initializer)
> + // but I don't think that is intended.
> -+ []{}; // OK?
> ++ []{}; // { dg-bogus "exposes TU-local entity" }
> +};
> +#endif
> +
> 2: 597f0448531 ! 2: b4e1e0e150f c++/modules: Implement ignored TU-local exposures
> @@ Metadata
> Author: Nathaniel Shead <nathanieloshead@gmail.com>
>
> ## Commit message ##
> - c++/modules: Implement ignored TU-local exposures
> + c++/modules: Ignore TU-local entities where necessary
>
> [basic.link] p14 lists a number of circumstances where a declaration
> naming a TU-local entity is not an exposure, notably the bodies of
> @@ Commit message
> prevent complicating the logic; I imagine this shouldn't ever come up
> though.
>
> - We also add a new warning, '-Wignored-exposures', to handle the case
> - where someone accidentally refers to a TU-local value from within a
> + We also add a new warning, '-Wtemplate-names-tu-local', to handle the
> + case where someone accidentally refers to a TU-local value from within a
> non-inline function template. This will compile without errors as-is,
> but any attempt to instantiate the decl will fail; this warning can be
> used to ensure that this doesn't happen. Unfortunately the warning has
> @@ Commit message
> instead just keeps the current (wrong) behaviour of non-exported
> entities not being visible to ADL at all.
>
> + Additionally, this patch doesn't attempt to ignore non-ODR uses of
> + constants in constexpr functions or templates. The obvious approach of
> + folding them early in 'mark_use' doesn't seem to work (for a variety of
> + reasons), so this leaves this to a later patch to implement, as it's at
> + least no worse than the current behaviour and easy enough to workaround.
> +
> + For completeness this patch adds a new xtreme-header testcase to ensure
> + that we have no regressions with regards to exposures of TU-local
> + declarations in the standard library header files. A more restrictive
> + test would be to do 'export extern "C++"' here, but unfortunately the
> + system headers on some targets declare TU-local entities, so we'll make
> + do with checking that at least the C++ standard library headers don't
> + refer to such entities.
> +
> gcc/c-family/ChangeLog:
>
> - * c.opt: New warning '-Wignored-exposures'.
> + * c.opt: New warning '-Wtemplate-names-tu-local'.
>
> gcc/cp/ChangeLog:
>
> @@ Commit message
> (TU_LOCAL_ENTITY_LOCATION): New accessor.
> (enum cp_tree_node_structure_enum): Add TS_CP_TU_LOCAL_ENTITY.
> (union GTY): Add tu_local_entity field.
> - * module.cc (enum tree_tag): New flag DB_IGNORED_EXPOSURE_BIT.
> - (depset::is_ignored_exposure): New accessor.
> + * module.cc (enum tree_tag): New flag DB_REFS_TU_LOCAL_BIT.
> (depset::has_defn): Override for TU-local entities.
> - (depset::hash::ignore_exposure): New field.
> + (depset::refs_tu_local): New accessor.
> + (depset::hash::ignore_tu_local): New field.
> (depset::hash::hash): Initialize it.
> (trees_out::tree_tag::tt_tu_local): New flag.
> (trees_out::writing_local_entities): New field.
> @@ Commit message
> (trees_out::find_tu_local_decl): New function.
> (trees_out::tree_node): Intercept TU-local entities and write
> placeholder values for them instead of normal streaming.
> - (trees_in::tree_node): Read TU-local entities; handle TU-local
> + (trees_in::tree_node): Handle TU-local entities and TU-local
> template results.
> (trees_out::write_function_def): Ignore exposures in non-inline
> function bodies.
> @@ Commit message
> (trees_in::read_class_def): Skip TU-local friends.
> (trees_out::write_definition): Record whether we're writing a
> decl which refers to TU-local entities.
> - (depset::hash::add_dependency): Handle ignored exposures.
> + (depset::hash::add_dependency): Only mark as exposure if we're not
> + ignoring TU-local entities.
> (depset::hash::find_dependencies): Use depset's own is_key_order
> function rather than delegating via walker. Pass whether the
> decl has ignored TU-local entities in its definition.
> (depset::hash::finalize_dependencies): Implement new warning
> - Wignored-exposures.
> + Wtemplate-names-tu-local.
> (module_state::intercluster_seed): Don't seed TU-local deps.
> (module_state::write_cluster): Pass whether the decl has ignored
> TU-local entities in its definition.
> @@ Commit message
>
> gcc/ChangeLog:
>
> - * doc/invoke.texi: Document -Wignored-exposures.
> + * doc/invoke.texi: Document -Wtemplate-names-tu-local.
>
> gcc/testsuite/ChangeLog:
>
> @@ Commit message
> * g++.dg/modules/internal-6.C: New test.
> * g++.dg/modules/internal-7_a.C: New test.
> * g++.dg/modules/internal-7_b.C: New test.
> + * g++.dg/modules/internal-8_a.C: New test.
> + * g++.dg/modules/xtreme-header-8.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> + Reviewed-by: Jason Merrill <jason@redhat.com>
>
> ## gcc/c-family/c.opt ##
> -@@ gcc/c-family/c.opt: Wif-not-aligned
> - C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
> - Warn when the field in a struct is not aligned.
> +@@ gcc/c-family/c.opt: Wtemplate-id-cdtor
> + C++ ObjC++ Var(warn_template_id_cdtor) Warning
> + Warn about simple-template-id in a constructor or destructor.
>
> -+Wignored-exposures
> -+C++ ObjC++ Var(warn_ignored_exposures) Warning EnabledBy(Wextra)
> -+Warn about ignored exposures of TU-local entities in a module.
> ++Wtemplate-names-tu-local
> ++C++ ObjC++ Var(warn_template_names_tu_local) Warning EnabledBy(Wextra)
> ++Warn about templates naming TU-local entities in a module.
> +
> - Wignored-qualifiers
> - C C++ Var(warn_ignored_qualifiers) Warning EnabledBy(Wextra)
> - Warn whenever type qualifiers are ignored.
> + Wterminate
> + C++ ObjC++ Warning Var(warn_terminate) Init(1)
> + Warn if a throw expression will always result in a call to terminate().
>
> ## gcc/cp/cp-objcp-common.cc ##
> @@ gcc/cp/cp-objcp-common.cc: cp_tree_size (enum tree_code code)
> @@ gcc/cp/cp-tree.h: check_constraint_info (tree t)
> it for unscoped enums. */
> #define DECL_MODULE_EXPORT_P(NODE) TREE_LANG_FLAG_3 (NODE)
>
> -+/* Represents a streamed-in translation-unit-local entity. Any use of this
> -+ should emit an error. */
> ++/* Represents a streamed-in translation-unit-local entity. Any use of
> ++ this node when instantiating a template should emit an error. */
> +struct GTY(()) tree_tu_local_entity {
> + struct tree_base base;
> + tree name;
> + location_t loc;
> +};
> +
> -+/* The name of a translation-unit local-entity. */
> ++/* The name of a translation-unit-local entity. */
> +#define TU_LOCAL_ENTITY_NAME(NODE) \
> + (((struct tree_tu_local_entity *)TU_LOCAL_ENTITY_CHECK (NODE))->name)
> +
> @@ gcc/cp/cp-tree.h: union GTY((desc ("cp_tree_node_structure (&%h)"),
>
> ## gcc/cp/module.cc ##
> @@ gcc/cp/module.cc: private:
> + DB_KIND_BITS = EK_BITS,
> + DB_DEFN_BIT = DB_KIND_BIT + DB_KIND_BITS,
> DB_IS_MEMBER_BIT, /* Is an out-of-class member. */
> - DB_TU_LOCAL_BIT, /* It is a TU-local entity. */
> +- DB_TU_LOCAL_BIT, /* It is a TU-local entity. */
> ++ DB_TU_LOCAL_BIT, /* Is a TU-local entity. */
> ++ DB_REFS_TU_LOCAL_BIT, /* Refers to a TU-local entity (but is not
> ++ necessarily an exposure.) */
> DB_EXPOSURE_BIT, /* Exposes a TU-local entity. */
> -+ DB_IGNORED_EXPOSURE_BIT, /* Will stream a TU-local entity. */
> DB_IMPORTED_BIT, /* An imported entity. */
> DB_UNREACHED_BIT, /* A yet-to-be reached entity. */
> - DB_HIDDEN_BIT, /* A hidden binding. */
> @@ gcc/cp/module.cc: public:
> public:
> bool has_defn () const
> @@ gcc/cp/module.cc: public:
> public:
> @@ gcc/cp/module.cc: public:
> {
> - return get_flag_bit<DB_EXPOSURE_BIT> ();
> + return get_flag_bit<DB_TU_LOCAL_BIT> ();
> }
> -+ bool is_ignored_exposure () const
> ++ bool refs_tu_local () const
> + {
> -+ return get_flag_bit<DB_IGNORED_EXPOSURE_BIT> ();
> ++ return get_flag_bit<DB_REFS_TU_LOCAL_BIT> ();
> + }
> - bool is_import () const
> + bool is_exposure () const
> {
> - return get_flag_bit<DB_IMPORTED_BIT> ();
> + return get_flag_bit<DB_EXPOSURE_BIT> ();
> @@ gcc/cp/module.cc: public:
> depset *current; /* Current depset being depended. */
> unsigned section; /* When writing out, the section. */
> bool reached_unreached; /* We reached an unreached entity. */
> -+ bool ignore_exposure; /* In a context where exposures are ignored. */
> ++ bool ignore_tu_local; /* In a context where referencing a TU-local
> ++ entity is not an exposure. */
>
> public:
> hash (size_t size, hash *c = NULL)
> : parent (size), chain (c), current (NULL), section (0),
> - reached_unreached (false)
> -+ reached_unreached (false), ignore_exposure (false)
> ++ reached_unreached (false), ignore_tu_local (false)
> {
> worklist.create (size);
> }
> @@ gcc/cp/module.cc: public:
> public:
> /* Serialize various definitions. */
> - void write_definition (tree decl);
> -+ void write_definition (tree decl, bool has_tu_local = false);
> ++ void write_definition (tree decl, bool refs_tu_local = false);
> void mark_declaration (tree decl, bool do_defn);
>
> private:
> @@ gcc/cp/module.cc: trees_out::tree_node (tree t)
> goto done;
>
> + /* Find TU-local entities and intercept streaming to instead write a
> -+ placeholder value; this way we don't need to expose such decls.
> ++ placeholder value; this way we don't need to emit such decls.
> + We only need to do this when writing a definition of an entity
> -+ that we know is an ignored exposure. */
> ++ that we know names a TU-local entity. */
> + if (!is_initial_scan () && writing_local_entities)
> + {
> + tree local_decl = NULL_TREE;
> @@ gcc/cp/module.cc: trees_in::tree_node (bool is_use)
>
> + case tt_tu_local:
> + {
> -+ /* A translation-unit local entity. */
> ++ /* A translation-unit-local entity. */
> + res = make_node (TU_LOCAL_ENTITY);
> + int tag = insert (res);
> +
> @@ gcc/cp/module.cc: void
> + is ignored for determining exposures. This should only matter
> + for templates (we don't emit the bodies of non-inline functions
> + to begin with). */
> -+ auto ovr = make_temp_override (dep_hash->ignore_exposure,
> ++ auto ovr = make_temp_override (dep_hash->ignore_tu_local,
> + !DECL_DECLARED_INLINE_P (decl));
> + tree_node (DECL_INITIAL (decl));
> + tree_node (DECL_SAVED_TREE (decl));
> @@ gcc/cp/module.cc: trees_in::read_function_def (tree decl, tree maybe_template)
> {
> + /* The initializer of a variable or variable template is ignored for
> + determining exposures. */
> -+ auto ovr = make_temp_override (dep_hash->ignore_exposure, VAR_P (decl));
> ++ auto ovr = make_temp_override (dep_hash->ignore_tu_local, VAR_P (decl));
> +
> tree init = DECL_INITIAL (decl);
> tree_node (init);
> @@ gcc/cp/module.cc: trees_out::write_class_def (tree defn)
> + {
> + /* Friend declarations in class definitions are ignored when
> + determining exposures. */
> -+ auto ovr = make_temp_override (dep_hash->ignore_exposure, true);
> ++ auto ovr = make_temp_override (dep_hash->ignore_tu_local, true);
>
> - /* Write the friend functions. */
> - for (tree friends = DECL_FRIENDLIST (defn);
> @@ gcc/cp/module.cc: trees_in::read_enum_def (tree defn, tree maybe_template)
>
> void
> -trees_out::write_definition (tree decl)
> -+trees_out::write_definition (tree decl, bool has_tu_local)
> ++trees_out::write_definition (tree decl, bool refs_tu_local)
> {
> + auto ovr = make_temp_override (writing_local_entities,
> -+ writing_local_entities || has_tu_local);
> ++ writing_local_entities || refs_tu_local);
> +
> if (streaming_p ())
> {
> @@ gcc/cp/module.cc: depset::hash::add_dependency (depset *dep)
> if (dep->is_tu_local ())
> - current->set_flag_bit<DB_EXPOSURE_BIT> ();
> + {
> -+ if (ignore_exposure)
> -+ current->set_flag_bit<DB_IGNORED_EXPOSURE_BIT> ();
> -+ else
> ++ current->set_flag_bit<DB_REFS_TU_LOCAL_BIT> ();
> ++ if (!ignore_tu_local)
> + current->set_flag_bit<DB_EXPOSURE_BIT> ();
> + }
>
> @@ gcc/cp/module.cc: depset::hash::find_dependencies (module_state *module)
> walker.decl_value (decl, current);
> if (current->has_defn ())
> - walker.write_definition (decl);
> -+ walker.write_definition (decl,
> -+ current->is_ignored_exposure ());
> ++ walker.write_definition (decl, current->refs_tu_local ());
> }
> walker.end ();
>
> @@ gcc/cp/module.cc: depset::hash::finalize_dependencies ()
> /* We should have emitted an error above. */
> gcc_checking_assert (explained);
> }
> -+ else if (warn_ignored_exposures && dep->is_ignored_exposure ())
> ++ else if (warn_template_names_tu_local
> ++ && dep->refs_tu_local () && !dep->is_tu_local ())
> + {
> + tree decl = dep->get_entity ();
> +
> @@ gcc/cp/module.cc: depset::hash::finalize_dependencies ()
> + for (depset *rdep : dep->deps)
> + if (!rdep->is_binding () && rdep->is_tu_local ())
> + {
> -+ tree exposed = rdep->get_entity ();
> ++ tree ref = rdep->get_entity ();
> + auto_diagnostic_group d;
> + if (warning_at (DECL_SOURCE_LOCATION (decl),
> -+ OPT_Wignored_exposures,
> ++ OPT_Wtemplate_names_tu_local,
> + "%qD refers to TU-local entity %qD and cannot "
> -+ "be instantiated in other TUs", decl, exposed))
> -+ is_tu_local_entity (exposed, /*explain=*/true);
> ++ "be instantiated in other TUs", decl, ref))
> ++ is_tu_local_entity (ref, /*explain=*/true);
> + break;
> + }
> + }
> @@ gcc/cp/module.cc: module_state::write_cluster (elf_out *to, depset *scc[], unsig
> sec.tree_node (decl);
> dump () && dump ("Writing definition %N", decl);
> - sec.write_definition (decl);
> -+ sec.write_definition (decl, b->is_ignored_exposure ());
> ++ sec.write_definition (decl, b->refs_tu_local ());
>
> if (!namer->has_defn ())
> namer = b;
> @@ gcc/cp/pt.cc: tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_d
>
> ## gcc/doc/invoke.texi ##
> @@ gcc/doc/invoke.texi: in the following sections.
> - -Weffc++ -Wno-elaborated-enum-base
> - -Wno-exceptions -Wextra-semi -Wno-global-module -Wno-inaccessible-base
> - -Wno-inherited-variadic-ctor -Wno-init-list-lifetime
> ---Winvalid-constexpr -Winvalid-imported-macros
> -+-Wignored-exposures -Winvalid-constexpr -Winvalid-imported-macros
> - -Wno-invalid-offsetof -Wno-literal-suffix
> - -Wmismatched-new-delete -Wmismatched-tags
> - -Wmultiple-inheritance -Wnamespaces -Wnarrowing
> -@@ gcc/doc/invoke.texi: the variable declaration statement.
> + -Woverloaded-virtual -Wno-pmf-conversions -Wself-move -Wsign-promo
> + -Wsized-deallocation -Wsuggest-final-methods
> + -Wsuggest-final-types -Wsuggest-override -Wno-template-body
> +--Wno-template-id-cdtor
> ++-Wno-template-id-cdtor -Wtemplate-names-tu-local
> + -Wno-terminate -Wno-vexing-parse -Wvirtual-inheritance
> + -Wno-virtual-move-assign -Wvolatile -Wzero-as-null-pointer-constant}
>
> - @end itemize
> +@@ gcc/doc/invoke.texi: template<typename T> struct S @{
> + @option{-Wtemplate-id-cdtor} is enabled by default with
> + @option{-std=c++20}; it is also enabled by @option{-Wc++20-compat}.
>
> -+@opindex Wignored-exposures
> -+@opindex Wno-ignored-exposures
> -+@item -Wignored-exposures
> ++@opindex Wtemplate-names-tu-local
> ++@opindex Wno-template-names-tu-local
> ++@item -Wtemplate-names-tu-local
> +Warn when a template body hides an exposure of a translation-unit-local
> +entity. In most cases, referring to a translation-unit-local entity
> +(such as an internal linkage declaration) within an entity that is
> +emitted into a module's CMI is an error. However, within the
> +initializer of a variable, or in the body of a non-inline function,
> -+this error is suppressed.
> ++this is not an exposure and no error is emitted.
> +
> +This can cause variable or function templates to accidentally become
> +unusable if they reference such an entity, because other translation
> +units that import the template will never be able to instantiate it.
> +This warning attempts to detect cases where this might occur.
> +
> -+This warning is enabled by @option{-Wextra}.
> ++This flag is enabled by @option{-Wextra}.
> +
> - @opindex Winvalid-constexpr
> - @opindex Wno-invalid-constexpr
> - @item -Winvalid-constexpr
> + @opindex Wterminate
> + @opindex Wno-terminate
> + @item -Wno-terminate @r{(C++ and Objective-C++ only)}
>
> ## gcc/testsuite/g++.dg/modules/internal-5_a.C (new) ##
> @@
> -+// { dg-additional-options "-fmodules-ts -Wignored-exposures" }
> ++// { dg-additional-options "-fmodules-ts -Wtemplate-names-tu-local" }
> +// { dg-module-cmi M }
> +// Ignore exposures in these cases
> +
> @@ gcc/testsuite/g++.dg/modules/internal-5_a.C (new)
> + internal_tmpl<T>();
> +}
> +template void function_tmpl<ok_inst_tag>();
> ++template <> void function_tmpl<ok_inst_tag*>() {}
> +
> +
> +// The initializer for a variable or variable template
> @@ gcc/testsuite/g++.dg/modules/internal-5_a.C (new)
> + internal_ovl(internal_x), internal_tmpl<T*>(), 0);
> +
> +template int var_tmpl<ok_inst_tag>;
> -+template int var_tmpl<ok_inst_tag*>;
> ++template <> int var_tmpl<ok_inst_tag*> = 0;
> +
> +export int& constant_ref = internal_x;
> +static_assert (&constant_ref == &internal_x);
> @@ gcc/testsuite/g++.dg/modules/internal-5_a.C (new)
> +// Any reference to a non-volatile const object or reference with internal or
> +// no linkage initialized with a constant expression that is not an ODR-use
> +static const int value = 123;
> -+static const int& ref = value;
> ++static const int& ref = 456;
> +static const internal_t internal {};
> ++void f(int) {}
> +export inline void no_odr_use() {
> + int x = value;
> + int y = ref;
> + int z = (internal, 0);
> ++
> ++ value;
> ++ bool b = value < value;
> ++ f(value);
> +}
>
> ## gcc/testsuite/g++.dg/modules/internal-5_b.C (new) ##
> @@ gcc/testsuite/g++.dg/modules/internal-5_b.C (new)
> + no_odr_use();
> +
> + function_tmpl<ok_inst_tag>();
> ++ function_tmpl<ok_inst_tag*>();
> + int b = var_tmpl<ok_inst_tag>;
> + int c = var_tmpl<ok_inst_tag*>;
> +
> @@ gcc/testsuite/g++.dg/modules/internal-6.C (new)
>
> ## gcc/testsuite/g++.dg/modules/internal-7_a.C (new) ##
> @@
> -+// { dg-additional-options "-fmodules-ts -Wignored-exposures" }
> ++// { dg-additional-options "-fmodules-ts -Wtemplate-names-tu-local" }
> +// Test streaming and instantiations of various kinds of exposures
> +
> +export module M;
> @@ gcc/testsuite/g++.dg/modules/internal-7_b.C (new)
> +}
> +
> +// { dg-error "instantiation exposes TU-local entity" "" { target *-*-* } 0 }
> +
> + ## gcc/testsuite/g++.dg/modules/internal-8_a.C (new) ##
> +@@
> ++// { dg-additional-options "-fmodules-ts -Wtemplate-names-tu-local" }
> ++// Non-ODR usages of const variables currently are erroneously
> ++// reported in templates and constexpr functions; this test
> ++// XFAILS them until we can implement a fix.
> ++
> ++export module M;
> ++
> ++namespace { struct internal_t {}; };
> ++static const int value = 123;
> ++static const int& ref = 456;
> ++static const internal_t internal {};
> ++
> ++constexpr void f(int) {}
> ++
> ++export constexpr
> ++void no_odr_use_cexpr() { // { dg-bogus "TU-local" "" { xfail *-*-* } }
> ++ int x = value;
> ++ int y = ref;
> ++ int z = (internal, 0);
> ++
> ++ value;
> ++ bool b = value < value;
> ++ f(value);
> ++}
> ++
> ++export template <typename T>
> ++void no_odr_use_templ() { // { dg-bogus "TU-local" "" { xfail *-*-* } }
> ++ int x = value;
> ++ int y = ref;
> ++ int z = (internal, 0);
> ++
> ++ value;
> ++ bool b = value < value;
> ++ f(value);
> ++}
> +
> + ## gcc/testsuite/g++.dg/modules/xtreme-header-8.C (new) ##
> +@@
> ++// PR c++/115126
> ++// { dg-additional-options "-fmodules-ts -Wtemplate-names-tu-local" }
> ++// { dg-module-cmi xstd }
> ++
> ++export module xstd;
> ++extern "C++" {
> ++ #include "xtreme-header.h"
> ++}
> 3: 49e06cc4065 ! 3: 918c037a0a9 c++/modules: Support anonymous namespaces in header units
> @@ Metadata
> Author: Nathaniel Shead <nathanieloshead@gmail.com>
>
> ## Commit message ##
> - c++/modules: Support anonymous namespaces in header units
> + c++/modules: Support unnamed namespaces in header units
>
> - A header unit may contain anonymous namespaces, and those declarations
> + A header unit may contain unnamed namespaces, and those declarations
> are exported (as with any declaration in a header unit). This patch
> ensures that such declarations are correctly handled.
>
> - The change to 'make_namespace_finish' is required so that if an
> - anonymous namespace is first seen by an import it is correctly handled
> - within 'add_imported_namespace'. I don't see any particular reason why
> - handling of anonymous namespaces here had to be handled separately
> - outside that function since these are the only two callers.
> + The change to 'make_namespace_finish' is required so that if an unnamed
> + namespace is first seen by an import it is correctly handled within
> + 'add_imported_namespace'. I don't see any particular reason why
> + handling of unnamed namespaces here had to be handled separately outside
> + that function since these are the only two callers.
>
> gcc/cp/ChangeLog:
>
> * module.cc (depset::hash::add_binding_entity): Also walk
> - anonymous namespaces.
> + unnamed namespaces.
> (module_state::write_namespaces): Adjust assertion.
> * name-lookup.cc (push_namespace): Move anon using-directive
> handling to...
> @@ Commit message
>
> gcc/testsuite/ChangeLog:
>
> - * g++.dg/modules/internal-8_a.H: New test.
> - * g++.dg/modules/internal-8_b.C: New test.
> + * g++.dg/modules/internal-9_a.H: New test.
> + * g++.dg/modules/internal-9_b.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> + Reviewed-by: Jason Merrill <jason@redhat.com>
>
> ## gcc/cp/module.cc ##
> @@ gcc/cp/module.cc: depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> @@ gcc/cp/name-lookup.cc: make_namespace_finish (tree ns, tree *slot, bool from_imp
> if (DECL_NAMESPACE_INLINE_P (ns) || !DECL_NAME (ns))
> emit_debug_info_using_namespace (ctx, ns, true);
> +
> ++ /* An unnamed namespace implicitly has a using-directive inserted so
> ++ that its contents are usable in the surrounding context. */
> + if (!DECL_NAMESPACE_INLINE_P (ns) && !DECL_NAME (ns))
> + add_using_namespace (NAMESPACE_LEVEL (ctx)->using_directives, ns);
> }
> @@ gcc/cp/name-lookup.cc: push_namespace (tree name, bool make_inline)
> }
>
>
> - ## gcc/testsuite/g++.dg/modules/internal-8_a.H (new) ##
> + ## gcc/testsuite/g++.dg/modules/internal-9_a.H (new) ##
> @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> @@ gcc/testsuite/g++.dg/modules/internal-8_a.H (new)
> +
> +namespace ns2 = ns;
>
> - ## gcc/testsuite/g++.dg/modules/internal-8_b.C (new) ##
> + ## gcc/testsuite/g++.dg/modules/internal-9_b.C (new) ##
> @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> -+import "internal-8_a.H";
> ++import "internal-9_a.H";
> +
> +int main() {
> + auto x2 = x;
> 4: 671a9de084e ! 4: 46d0ecf964c c++/modules: Check linkage for exported declarations
> @@ Commit 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 ##
> @@ gcc/cp/cp-tree.h: extern void set_originating_module (tree, bool friend_p = false);
> @@ gcc/cp/module.cc: set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNU
> }
>
> - 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
> ++ /* 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 ())
> @@ gcc/cp/name-lookup.cc: push_namespace (tree name, bool make_inline)
> - "exporting namespace with internal linkage");
> + {
> + if (name)
> -+ error_at (input_location,
> -+ "exporting namespace %qD with internal linkage", ns);
> ++ {
> ++ 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 anonymous namespace");
> ++ error_at (input_location, "exporting unnamed namespace");
> + }
> }
> if (module_purview_p ())
> @@ gcc/testsuite/g++.dg/modules/export-3.C: namespace {
> }
>
> -export namespace {} // { dg-error "internal linkage" }
> -+export namespace {} // { dg-error "exporting anonymous namespace" }
> ++export namespace {} // { dg-error "exporting unnamed namespace" }
>
> ## gcc/testsuite/g++.dg/modules/export-6.C (new) ##
> @@
> @@ gcc/testsuite/g++.dg/modules/export-6.C (new)
> +#endif
> +}
> +
> -+export namespace {} // { dg-error "exporting anonymous namespace" }
> ++export namespace {} // { dg-error "exporting unnamed namespace" }
> +export namespace ns2 = ns; // { dg-error "internal linkage" }
>
> ## libcc1/libcp1plugin.cc ##
> 5: 0923d54daf1 ! 5: 3470fe5b429 c++/modules: Validate external linkage definitions in header units [PR116401]
> @@ Commit message
> * g++.dg/modules/hdr-2.H: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> + Reviewed-by: Jason Merrill <jason@redhat.com>
>
> ## gcc/cp/decl.cc ##
> @@ gcc/cp/decl.cc: grokfndecl (tree ctype,
> have one: the restriction that you can't repeat a deduction guide
> makes them more like a definition anyway. */
> DECL_INITIAL (decl) = void_node;
> ++ /* But to ensure that external-linkage deduction guides in header units
> ++ don't fall afoul of [module.import] p6, mark them as inline to give
> ++ the same effect as them not having a definition. */
> + DECL_DECLARED_INLINE_P (decl) = true;
> break;
> default:
> 6: 0f78398918f < -: ----------- c++/modules: Add testcase for standard-library exposures [PR115126]
> --
> 2.46.0
>
More information about the Gcc-patches
mailing list