On 10/24/24 3:25 AM, Nathaniel Shead wrote:
I wasn't sure whether I should include the ambiguity checking logic from
process_partial_specialization; we don't do this anywhere else in the
modules handling code that I could see so I left it out for now.
The relevant bit in the standard seems to be
https://eel.is/c++draft/temp#spec.partial.general-1
"A partial specialization shall be reachable from any use of a template
specialization that would make use of the partial specialization as the
result of an implicit or explicit instantiation; no diagnostic is required."
So we aren't required to diagnose an imported partial spec after an
instantiation. It's not entirely clear whether the ambiguity checking
in https://eel.is/c++draft/temp#spec.partial.match-1.2 qualifies as
"make use", but that seems a natural interpretation.
So checking these on import seems optional, but could also be helpful in
finding subtle bugs.
I could also rework process_partial_specialization to call the new
create_mergeable_partial_spec function if you prefer, to reduce code
duplication, though the early exit for VAR_DECL and the call to
associate_classtype_constraints that relies on global state complicated
it enough I didn't bother for this patch.
That sounds desirable, if we need to create the mergeable partial spec
at all.
Finally, I'm not entirely sure whether the choice of DECL_TEMPLATE_PARMS
that I'm providing is correct, especially considering this hunk:
/* Give template template parms a DECL_CONTEXT of the template
for which they are a parameter. */
for (i = 0; i < ntparms; ++i)
{
tree parm = TREE_VALUE (TREE_VEC_ELT (inner_parms, i));
if (TREE_CODE (parm) == TEMPLATE_DECL)
DECL_CONTEXT (parm) = tmpl;
}
I haven't been able to produce any testcase that relies on this context
setting to cause a failure, though, and there doesn't seem to be one in
the testsuite for the matching hunk in process_partial_specialization.
Any hints on where this might be needed?
I believe that's for the benefit of coerce_template_args_for_ttp.
In some cases, when we go to import a partial specialisation there might
already be an incomplete implicit instantiation in the specialisation
table. This causes ICEs described in the linked PR as we now have two
separate matching specialisations for this same arguments.
This issue doesn't appear to happen for variable templates as it doesn't
appear that we can create incomplete specialisations in the same way.
This patch attempts to solve the issue by retrofitting the existing
implicit instantiation as a partial specialisation (similarly to how
maybe_process_partial_specialization operates) and then relying on the
existing merging logic to fill in the definition of the type. This
works because for types almost all relevant details are streamed and
merged in 'read_class_def'.
I find it surprising that we would need to build a partial
specialization to merge with, when in a single TU we have no trouble
merging a partial specialization with an existing implicit
instantiation. Why can't we do that in this case as well?
As an optimisation, we also mark the newly retrofitted specialisation as
imported, since there isn't any useful information on the declaration
that will be provided from this TU anyway, and so we don't need to write
the decl into our own BMI but can instead just seed it as an import.
Agreed.
Jason
