From 584731ecedf09c2c067913c4af9ed0a30cf19e8d Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Thu, 1 Apr 2021 05:25:53 -0700 Subject: [PATCH] c++: inter-cluster import order [PR 99283] I finally managed to reduce the testcase without hitting other bugs. This problem is caused by discovering a duplicate in the middle of reading in the entity in question. I had thougt the import seeding at the beginning of a cluster prevented that, but it is insufficient. Specifically an earlier cluster in the same module can cause the import of a duplicate. Although clusters within a module are well-ordered, there is no ordering between clusters of one module and clusters of another module. And thus we can get duplicate declaration loops. This prevents the problem by also seeding references to earlier clusters in the same module. As the FIXME notes, it is sufficient to reference a single entity in any particular earlier cluster, plus, we also could determine the implicit dependencies and prune that seeding even further. I do not do that -- it decrease the loading that will happen, but would reduce the serialization size. As ever, let's get correctness first. PR c++/99283 gcc/cp/ * module.cc (trees_out::decl_node): Adjust importedness reference assert. (module_state::intercluster_seed): New. Seed both imports and inter-cluster references. Broken out of ... (module_state::write_cluster): ... here. Call it. gcc/testsuite/ * g++.dg/modules/pr99283-6.h: New. * g++.dg/modules/pr99283-6_a.H: New. * g++.dg/modules/pr99283-6_b.H: New. * g++.dg/modules/pr99283-6_c.C: New. * g++.dg/modules/hdr-init-1_c.C: Adjust scan. * g++.dg/modules/indirect-3_c.C: Adjust scan. * g++.dg/modules/indirect-4_c.C: Adjust scan. * g++.dg/modules/lambda-3_b.C: Adjust scan. * g++.dg/modules/late-ret-3_c.C: Adjust scan. * g++.dg/modules/pr99425-1_b.H: Adjust scan. * g++.dg/modules/pr99425-1_c.C: Adjust scan. --- gcc/cp/module.cc | 63 ++++---- gcc/testsuite/g++.dg/modules/hdr-init-1_c.C | 4 +- gcc/testsuite/g++.dg/modules/indirect-3_c.C | 2 +- gcc/testsuite/g++.dg/modules/indirect-4_c.C | 2 +- gcc/testsuite/g++.dg/modules/lambda-3_b.C | 6 +- gcc/testsuite/g++.dg/modules/late-ret-3_c.C | 2 +- gcc/testsuite/g++.dg/modules/pr99283-6.h | 23 +++ gcc/testsuite/g++.dg/modules/pr99283-6_a.H | 33 ++++ gcc/testsuite/g++.dg/modules/pr99283-6_b.H | 164 ++++++++++++++++++++ gcc/testsuite/g++.dg/modules/pr99283-6_c.C | 10 ++ gcc/testsuite/g++.dg/modules/pr99425-1_b.H | 2 +- gcc/testsuite/g++.dg/modules/pr99425-1_c.C | 4 +- 12 files changed, 278 insertions(+), 37 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/pr99283-6.h create mode 100644 gcc/testsuite/g++.dg/modules/pr99283-6_a.H create mode 100644 gcc/testsuite/g++.dg/modules/pr99283-6_b.H create mode 100644 gcc/testsuite/g++.dg/modules/pr99283-6_c.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index fab6b573d249..c87ddd16a80d 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -3570,6 +3570,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { unsigned, unsigned *crc_ptr); bool read_namespaces (unsigned); + void intercluster_seed (trees_out &sec, unsigned index, depset *dep); unsigned write_cluster (elf_out *to, depset *depsets[], unsigned size, depset::hash &, unsigned *counts, unsigned *crc_ptr); bool read_cluster (unsigned snum); @@ -8548,8 +8549,7 @@ trees_out::decl_node (tree decl, walk_kind ref) gcc_checking_assert (index == ~import_entity_index (decl)); #if CHECKING_P - if (importedness) - gcc_assert (!import == (importedness < 0)); + gcc_assert (!import || importedness >= 0); #endif i (tt_entity); u (import); @@ -14419,7 +14419,33 @@ enum ct_bind_flags cbf_wrapped = 0x8, /* ... that is wrapped. */ }; -/* Write the cluster of depsets in SCC[0-SIZE). */ +/* DEP belongs to a different cluster, seed it to prevent + unfortunately timed duplicate import. */ +// FIXME: QOI For inter-cluster references we could just only pick +// one entity from an earlier cluster. Even better track +// dependencies between earlier clusters + +void +module_state::intercluster_seed (trees_out &sec, unsigned index_hwm, depset *dep) +{ + if (dep->is_import () + || dep->cluster < index_hwm) + { + tree ent = dep->get_entity (); + if (!TREE_VISITED (ent)) + { + sec.tree_node (ent); + dump (dumper::CLUSTER) + && dump ("Seeded %s %N", + dep->is_import () ? "import" : "intercluster", ent); + } + } +} + +/* Write the cluster of depsets in SCC[0-SIZE). + dep->section -> section number + dep->cluster -> entity number + */ unsigned module_state::write_cluster (elf_out *to, depset *scc[], unsigned size, @@ -14431,6 +14457,7 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size, trees_out sec (to, this, table, table.section); sec.begin (); + unsigned index_lwm = counts[MSC_entities]; /* Determine entity numbers, mark for writing. */ dump (dumper::CLUSTER) && dump ("Cluster members:") && (dump.indent (), true); @@ -14484,10 +14511,10 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size, } dump (dumper::CLUSTER) && (dump.outdent (), true); - /* Ensure every imported decl is referenced before we start - streaming. This ensures that we never encounter the situation - where this cluster instantiates some implicit member that - importing some other decl causes to be instantiated. */ + /* Ensure every out-of-cluster decl is referenced before we start + streaming. We must do both imports *and* earlier clusters, + because the latter could reach into the former and cause a + duplicate loop. */ sec.set_importing (+1); for (unsigned ix = 0; ix != size; ix++) { @@ -14505,30 +14532,14 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size, depset *bind = dep->deps[ix]; if (bind->get_entity_kind () == depset::EK_USING) bind = bind->deps[1]; - if (bind->is_import ()) - { - tree import = bind->get_entity (); - if (!TREE_VISITED (import)) - { - sec.tree_node (import); - dump (dumper::CLUSTER) - && dump ("Seeded import %N", import); - } - } + + intercluster_seed (sec, index_lwm, bind); } /* Also check the namespace itself. */ dep = dep->deps[0]; } - if (dep->is_import ()) - { - tree import = dep->get_entity (); - if (!TREE_VISITED (import)) - { - sec.tree_node (import); - dump (dumper::CLUSTER) && dump ("Seeded import %N", import); - } - } + intercluster_seed (sec, index_lwm, dep); } } sec.tree_node (NULL_TREE); diff --git a/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C b/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C index efcc48543140..2a103c3d0bb7 100644 --- a/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C +++ b/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C @@ -17,8 +17,8 @@ int main () // { dg-final { scan-lang-dump-times {Reading 1 initializers} 2 module } } -// { dg-final { scan-lang-dump {Read:-1's named merge key \(new\) var_decl:'::var'} module } } +// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(new\) var_decl:'::var'} module } } // { dg-final { scan-lang-dump-times {Reading definition var_decl '::var@[^\n]*/hdr-init-1_a.H:1'} 2 module } } -// { dg-final { scan-lang-dump {Read:-1's named merge key \(matched\) var_decl:'::var'} module } } +// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(matched\) var_decl:'::var'} module } } diff --git a/gcc/testsuite/g++.dg/modules/indirect-3_c.C b/gcc/testsuite/g++.dg/modules/indirect-3_c.C index 9c5cb230ad2f..ec2fc7683731 100644 --- a/gcc/testsuite/g++.dg/modules/indirect-3_c.C +++ b/gcc/testsuite/g++.dg/modules/indirect-3_c.C @@ -19,6 +19,6 @@ int main () // { dg-final { scan-lang-dump-not {Instantiation:-[0-9]* function_decl:'::foo::X@foo:.::frob@.()<0x0>'} module } } // { dg-final { scan-lang-dump {Lazily binding '::bar@bar:.::toto'@'bar' section:} module } } -// { dg-final { scan-lang-dump {>Loading entity foo\[1\] section:1} module } } +// { dg-final { scan-lang-dump {>Loading entity foo\[.\] section:1} module } } // { dg-final { scan-lang-dump {Imported:-[0-9]* template_decl:'::foo@foo:.::template TPL@foo:.'@foo} module } } // { dg-final { scan-lang-dump {Reading definition type_decl '::foo@foo:.::TPL@bar:.<0x0>'} module } } diff --git a/gcc/testsuite/g++.dg/modules/indirect-4_c.C b/gcc/testsuite/g++.dg/modules/indirect-4_c.C index 7efcc115e71f..d55a2216fb33 100644 --- a/gcc/testsuite/g++.dg/modules/indirect-4_c.C +++ b/gcc/testsuite/g++.dg/modules/indirect-4_c.C @@ -10,7 +10,7 @@ int main () } // { dg-final { scan-lang-dump {Lazily binding '::bar@bar:.::quux'@'bar' section:} module } } -// { dg-final { scan-lang-dump {>Loading entity foo\[2\] section:1} module } } +// { dg-final { scan-lang-dump {>Loading entity foo\[.\] section:1} module } } // { dg-final { scan-lang-dump {Imported:-[0-9]* template_decl:'::foo@foo:.::template TPL@foo:.'@foo} module } } // { dg-final { scan-lang-dump {Reading definition function_decl '::foo@foo:.::TPL@bar:.<0x1>::frob@bar:.<0x2>'} module } } diff --git a/gcc/testsuite/g++.dg/modules/lambda-3_b.C b/gcc/testsuite/g++.dg/modules/lambda-3_b.C index 25a418bb44ae..17afd9677457 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-3_b.C +++ b/gcc/testsuite/g++.dg/modules/lambda-3_b.C @@ -4,6 +4,6 @@ import "lambda-3_a.H"; // { dg-final { scan-lang-dump-not {merge key \(new\)} module } } -// { dg-final { scan-lang-dump {Read -1\[0\] matched attached decl '::template ._anon_0<#unnamed#>'} module } } -// { dg-final { scan-lang-dump {Read -1\[0\] matched attached decl '::._anon_2'} module } } -// { dg-final { scan-lang-dump {Read -1\[0\] matched attached decl '::._anon_1'} module } } +// { dg-final { scan-lang-dump {Read -[0-9]*\[0\] matched attached decl '::template ._anon_0<#unnamed#>'} module } } +// { dg-final { scan-lang-dump {Read -[0-9]*\[0\] matched attached decl '::._anon_2'} module } } +// { dg-final { scan-lang-dump {Read -[0-9]*\[0\] matched attached decl '::._anon_1'} module } } diff --git a/gcc/testsuite/g++.dg/modules/late-ret-3_c.C b/gcc/testsuite/g++.dg/modules/late-ret-3_c.C index fae956542f99..8ab23a9e3385 100644 --- a/gcc/testsuite/g++.dg/modules/late-ret-3_c.C +++ b/gcc/testsuite/g++.dg/modules/late-ret-3_c.C @@ -19,4 +19,4 @@ int main () return 0; } -// { dg-final { scan-lang-dump {Read:-1's named merge key \(matched\) template_decl:'::template Foo'\n Deduping '::template Foo@[^\n]*/late-ret-3_a.H:.'\n} module } } +// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(matched\) template_decl:'::template Foo'\n Deduping '::template Foo@[^\n]*/late-ret-3_a.H:.'\n} module } } diff --git a/gcc/testsuite/g++.dg/modules/pr99283-6.h b/gcc/testsuite/g++.dg/modules/pr99283-6.h new file mode 100644 index 000000000000..383b66c081e0 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99283-6.h @@ -0,0 +1,23 @@ +template +struct conditional; + +template struct incrementable_traits; + +template +struct __iter_traits_impl; + +class __max_diff_type; + +template +concept weakly_incrementable + = __is_same (__iter_traits_impl<_Iter, incrementable_traits<_Iter>>, + __max_diff_type); + +template +class reverse_iterator +{ +public: + using iterator_concept + = typename conditional, + int, int>::type; +}; diff --git a/gcc/testsuite/g++.dg/modules/pr99283-6_a.H b/gcc/testsuite/g++.dg/modules/pr99283-6_a.H new file mode 100644 index 000000000000..176ad9e9e8cb --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99283-6_a.H @@ -0,0 +1,33 @@ +// { dg-additional-options {-std=c++2a -fmodule-header} } +// { dg-module-cmi {} + +#include "pr99283-6.h" + +template +constexpr bool + operator<(const reverse_iterator<_IteratorL>& __x, + const reverse_iterator<_IteratorR>& __y); + +template + struct numeric_limits; + +class __max_size_type +{ +public: + template + constexpr + __max_size_type(_Tp __i) noexcept + : _M_val(__i), _M_msb(__i < 0) + { } + + using __rep = __UINT64_TYPE__; +private: + __rep _M_val = 0; + unsigned _M_msb:1 = 0; +}; + +class __max_diff_type +{ +private: + __max_size_type _M_rep = 0; +}; diff --git a/gcc/testsuite/g++.dg/modules/pr99283-6_b.H b/gcc/testsuite/g++.dg/modules/pr99283-6_b.H new file mode 100644 index 000000000000..123f35346738 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99283-6_b.H @@ -0,0 +1,164 @@ +// { dg-additional-options {-std=c++2a -fmodule-header} } +// { dg-module-cmi {} + +#include "pr99283-6.h" + +template +void __addressof(_Tp& __r) ; + +template +struct integral_constant +{ + static constexpr _Tp value = __v; +}; + +template +constexpr _Tp integral_constant<_Tp, __v>::value; + +typedef integral_constant true_type; +typedef integral_constant false_type; + +template +struct __and_ + : public conditional<_B1::value, _B2, _B1>::type +{ }; + +template +struct is_convertible + : public true_type +{ }; + +template +struct enable_if +{ }; + +template +struct enable_if +{ typedef _Tp type; }; + +template +using __enable_if_t = typename enable_if<_Cond, _Tp>::type; + +template +using _Require = __enable_if_t<__and_::value>; + +template +struct conditional +{ typedef _Iftrue type; }; + +template struct iterator_traits; + + +template +constexpr bool + operator!=(const reverse_iterator<_IteratorL>& __x, + const reverse_iterator<_IteratorR>& __y); + +typedef __INT64_TYPE__ int64_t; +typedef int64_t intmax_t; + +template +struct ratio +{ +}; + +namespace chrono +{ +template +struct duration; + +template +constexpr _ToDur + duration_cast(const duration<_Rep>& __d) +{ + typedef typename _ToDur::rep __to_rep; + return _ToDur(static_cast<__to_rep>(static_cast(__d.count()))); +} + +template +struct duration +{ + +public: + using rep = _Rep; + + constexpr duration() = default; + + duration(const duration&) = default; + + template, + true_type>> + constexpr explicit duration(const _Rep2& __rep) + : __r (__rep) {} + + ~duration() = default; + duration& operator=(const duration&) = default; + + rep count() const; + +private: + rep __r; +}; + +using seconds = duration; + +template +struct time_point +{ + typedef _Dur duration; + + duration time_since_epoch() const; + +private: + duration __d; +}; + +struct system_clock +{ + typedef chrono::seconds duration; + + typedef chrono::time_point time_point; + + static void + to_time_t(const time_point& __t) noexcept + { + duration_cast + (__t.time_since_epoch()).count(); + } +}; + +} + +template +class allocator; + +template +constexpr void + __destroy(_ForwardIterator __first, _ForwardIterator __last) +{ + for (; __first != __last; ++__first) + __addressof (*__first); +} + +template +struct allocator_traits +{ +private: + template + static constexpr void + _S_destroy(_Alloc2&, _Tp* __p, ...) + noexcept + { + __destroy(__p); + } +}; + +template +struct allocator_traits> +{ + using value_type = _Tp; + using pointer = _Tp*; +}; + +import "pr99283-6_a.H"; diff --git a/gcc/testsuite/g++.dg/modules/pr99283-6_c.C b/gcc/testsuite/g++.dg/modules/pr99283-6_c.C new file mode 100644 index 000000000000..9492554617d9 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99283-6_c.C @@ -0,0 +1,10 @@ +// { dg-additional-options {-std=c++2a -fmodules-ts} } + +import "pr99283-6_b.H"; + +template +struct __allocated_ptr +{ + using value_type = allocator_traits<_Alloc>; +}; + diff --git a/gcc/testsuite/g++.dg/modules/pr99425-1_b.H b/gcc/testsuite/g++.dg/modules/pr99425-1_b.H index 98303a0c6872..53d28b4ef5e9 100644 --- a/gcc/testsuite/g++.dg/modules/pr99425-1_b.H +++ b/gcc/testsuite/g++.dg/modules/pr99425-1_b.H @@ -15,5 +15,5 @@ inline void widget (Cont parm) ssize (parm); } -// { dg-final { scan-lang-dump {Read:-[0-9]*'s alias spec merge key \(new\) type_decl:'::make_signed_t'\n ... Read:-[0-9]*'s type spec merge key \(new\) type_decl:'::make_signed'\n Read:-1's named merge key \(matched\) template_decl:'::template ssize'} module } } +// { dg-final { scan-lang-dump {Read:-[0-9]*'s alias spec merge key \(new\) type_decl:'::make_signed_t'\n ... Read:-[0-9]*'s type spec merge key \(new\) type_decl:'::make_signed'\n Read:-[0-9]*'s named merge key \(matched\) template_decl:'::template ssize'} module } } diff --git a/gcc/testsuite/g++.dg/modules/pr99425-1_c.C b/gcc/testsuite/g++.dg/modules/pr99425-1_c.C index 28ef3a1ff304..77a35a88b391 100644 --- a/gcc/testsuite/g++.dg/modules/pr99425-1_c.C +++ b/gcc/testsuite/g++.dg/modules/pr99425-1_c.C @@ -7,5 +7,5 @@ void frob (Cont parm) ssize (parm); } -// { dg-final { scan-lang-dump {Read:-1's named merge key \(new\) template_decl:'::template ssize'} module } } -// { dg-final { scan-lang-dump {Read:-1's named merge key \(matched\) template_decl:'::template ssize'} module } } +// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(new\) template_decl:'::template ssize'} module } } +// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(matched\) template_decl:'::template ssize'} module } } -- 2.47.2