https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127694
--- Comment #1 from Davud Durakovic <me at davud dot io> ---
Here's my little analysis on what causes this issue.
The repro was reduced with C-Vise and then by hand, from a real project. It had
an `absl::flat_hash_set` member in a class in a module partition, with a header
for Google's Abseil library included in that same partition's GMF.
The module.cc referenced below is `cp/module.cc` at commit a87de1ad7.
When `bar.cppm` starts writing `mod1`, it re-streams the partition's entities.
`enable_if_t<decltype(v)>`, `Conversion` and `enable_if<decltype(v)>` form one
cluster, and `enable_if_t<decltype(v)>` comes first:
```
Writing section:4 4 depsets
Cluster members:
[0]=specialization declaration '::enable_if_t@mod1:part:1<#null#>'
[1]=decl declaration '::template Conversion@mod1:part:1'
[2]=specialization declaration '::enable_if@mod1:part:1<#null#>'
[3]=binding '::Conversion'
```
`enable_if_t<decltype(v)>`'s merge key contains `decltype(v)`. Here,
`Conversion` is reached first via `v`'s DECL_CONTEXT, so `Conversion` is
written inline inside `enable_if_t<decltype(v)>`'s merge key. `Conversion`'s
own merge key contains its return type `enable_if_t<decltype(v)>`. The
`type_node` function writes that as `tt_typedef_type` alongside a backref to
`enable_if_t<decltype(v)>`'s TYPE_DECL. However, `enable_if_t<decltype(v)>`'s
type has not been visited yet, which causes the assert at `module.cc:9735` to
fail:
```
.7. Writing:-11's named merge key (decl) template_decl:'::template
Conversion@mod1:part:1'
.8. Written:-20 identifier:'Conversion'
.8. Writing typedef type_decl:'::enable_if_t@mod1:part:1<#null#>'
.9. Wrote backref:-9 type_decl:'::enable_if_t@mod1:part:1<#null#>'
.8. Wrote typedef type_decl:'::enable_if_t@mod1:part:1<#null#>'
<assert>
```
Without checking, the same data gets written but the compiler never aborts.
When the compiler reads the CMI while compiling `foo`, the case for
`tt_typedef_type` inside `trees_in::tree_node` returns the `TREE_TYPE` of
`enable_if_t<decltype(v)>`'s TYPE_DECL. At that point it is still NULL, because
`enable_if_t<decltype(v)>` is still being read (as stated by the comment at
`module.cc:8945`). The NULL ends up as `Conversion`'s return type and reaches
`cp_build_function_type`, which calls `build_function_type`, which causes a
segmentation fault.
The cluster itself isn't properly ordered because `sort_cluster` expects a
multi-entity cluster to have exactly one "entry" entity, which is moved to the
front of the sorted SCC (as stated in comment at `module.cc:16348`). This
entity is marked with `DB_ENTRY_BIT` in `add_dependency` (`module.cc:14919 -
14929`). The first time a depset is reached while walking another depset's
merge key, `current` becomes the entry, while `current` and `dep` is marked
with `DB_MAYBE_RECURSIVE_BIT`.
In the GMF case, `Conversion` isn't inserted into depset table worklist,
because `add_binding_entity` disregards decls that are not in module purview
(`module.cc:15017`).Consequently, `Conversion`'s depset is first created when
`find_dependencies` walks through `Bar::Func`'s merge key, whose parameter type
is `decltype(Conversion<Args>)`. So:
1. `current = Func` and `dep = Conversion` while inside a merge key. `Func`
gets assigned the `DB_ENTRY_BIT`, and both get `DB_MAYBE_RECURSIVE_BIT`.
2. Later, `Conversion`'s merge key reaches `enable_if_t<decltype(v)>`.
`Conversion` already has MAYBE_RECURSIVE, so neither gets the entry bit.
`Func`, however, is in `Bar`'s cluster, not Conversion's, so its entry bit has
no effect. The cluster {`enable_if_t<decltype(v)>`, `Conversion`,
`enable_if<decltype(v)>`} ends up with no entry. The issue is that the entry
bit heuristic assumes that `current` and `dep` end up in the same cluster, but
clusters aren't known yet during `find_dependencies`.
In the module dump of `bar.cppm`:
```
Mergeable 0 is '::enable_if_t@mod1:part:1<#null#>'
Mergeable 1 is '::template Conversion@mod1:part:1' (tight)
Mergeable 2 is '::enable_if@mod1:part:1<#null#>' (tight)
```
Similarly, in the dump for `baz.cppm`, Conversion has the same missing entry
bit, which is still taken by `Func`:
```
Mergeable 0 is '::template Conversion'
Mergeable 1 is '::enable_if_t<#null#>' (tight)
Mergeable 2 is '::enable_if<#null#>'
...
Wrote section:4 named-by:'::template Conversion'
...
Mergeable 2 is '::Namespace::Bar::template Func' (entry)
```
With no entry, the order within the cluster is whatever the `connect()`
function produces. `connect()` sorts by using `depset_cmp`, which in turn sorts
first by entity kind, then by `DECL_UID`, and starts Tarjan from the end, so it
starts from the specialization with the higher `DECL_UID`. (the starting order
for Tarjan matters since nodes come out in reverse visit order)
- In baz, `enable_if_t<decltype(v)>` has the higher UID, which gives the SCC of
[`Conversion`, `enable_if_t<decltype(v)>`, `enable_if<decltype(v)>`].
- In bar, `enable_if<decltype(v)>` has the higher UID, which gives
[`enable_if_t<decltype(v)>`, `Conversion`, `enable_if<decltype(v)>`].
I am not 100% sure why the UIDs differ, but I assume that in `bar.cppm` the
decls are created by the CMI reader in streaming order, not in parsing order.
This would also explain why a partition is needed, since only the primary
interface re-streams the partition's entities as its own.
In the purview variant, `Conversion` is inserted into the worklist by
`add_namespace_entities`. Here, the iteration order of
`DECL_NAMESPACE_BINDINGS` (which is a hash table keyed by
identifier) decides whether `Conversion` is walked before `Bar`, and therefore
before `Func`'s merge key reaches it.
With the function name `Conversion`, it is walked first and becomes the entry
entity. The module dump shows
`Mergeable 1 is '::Namespace::template Conversion@mod1:part:1' (tight) (entry)`
for bar in the purview case. The swap moves it to the front of the cluster,
which avoids the error.
With the name `q`, `Func` is walked first. That creates the same situation as
in the GMF case, where the module dump outputs
`Mergeable 1 is '::Namespace::template q@mod1:part:1' (tight)`,
with no entry bit, resulting in the same error
I verified this with a hack, by replacing
```
if (dep->is_entry ())
```
with
```
if (dep->is_entry ()
|| (TREE_CODE (dep->get_entity ()) == TEMPLATE_DECL
&& id_equal (DECL_NAME (dep->get_entity ()), "Conversion")))
```
in `module.cc:16380`. This resulted in no crash in the GMF case (obviously
given the function is still named `Conversion`).
Anyways, wall of text over. I'd like to try writing a fix for this if the bug
is confirmed, unless a maintainer would want to do the same.
