On 8/6/24 8:03 AM, Nathaniel Shead wrote:
On Fri, Jul 26, 2024 at 01:17:57PM -0400, Jason Merrill wrote:
On 7/26/24 12:52 AM, Nathaniel Shead wrote:
On Tue, Jul 23, 2024 at 04:17:22PM -0400, Jason Merrill wrote:
On 6/15/24 10:29 PM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
This probably isn't the most efficient approach, since we need to do
name lookup to find deduction guides for a type which will also
potentially do a bunch of pointless lazy loading from imported modules,
but I wasn't able to work out a better approach without completely
reworking how deduction guides are stored and represented.
Indeed. We likely want to find them more directly from the template; it's
not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we could put
them in an internal attribute or a separate hash table.
I had a go at exploring some other representations of deduction guides
but I didn't really land anywhere; most things I tried would require
reimplementing a lot of the merging logic handled by name lookup
currently. Potentially just having an additional hash table for
deduction guides maintained by pushdecl would work but that felt like
unnecessary duplication, I'm not sure that the benefits outweight the
cost of the name lookup. (But I haven't measured this.)
-- >8 --
Deduction guides are represented as 'normal' functions currently, and
have no special handling in modules. However, this causes some issues;
by [temp.deduct.guide] a deduction guide is not found by normal name
lookup and instead all reachable deduction guides for a class template
should be considered, but this does not happen currently.
To solve this, this patch ensures that all deduction guides are
considered exported to ensure that they are always visible to importers
if they are reachable. Another alternative here would be to add a new
kind of "all reachable" flag to name lookup, but that is complicated by
some difficulties in handling GM entities; this may be a better way to
go if more kinds of entities end up needing this handling, however.
Another issue here is that because deduction guides are "unrelated"
functions, they will usually get discarded from the GMF, so this patch
ensures that when finding dependencies, GMF deduction guides will also
have bindings created. We do this in find_dependencies so that we don't
unnecessarily create bindings for GMF deduction guides that are never
reached; for consistency we do this for *all* deduction guides, not just
GM ones.
If you fixed the dependency calculation, why do they also need to be
exported?
Deduction guides aren't found using normal name lookup, but any
reachable deduction guide must be considered. This means that even if
the module interface exports no declarations whatsoever, a deduction
guide declared in the module purview must still be considered by
importers.
Ah, I was missing the name lookup issue.
The other option I've considered is adding a new "ANY_REACHABLE" flag to
name lookup which would also consider non-exported reachable decls. On
further consideration I might actually go this way; I've been thinking
about how to resolve some issues adjacent to supporting textual
redefinitions that I believe this will be necessary for anyway, and we
can probably use this in tsubst_friend_class as well rather than the
current relatively ad-hoc solution.
There's also my hack in lookup_elaborated_type for ABI namespace types.
I'm not sure that it should be necessary to do this for redefinitions,
though; what's the advantage over merging in check_module_override (apart
from that needing to be fixed)?
That said, I've realised that this patch isn't completely sufficient
anyway; consider:
// m.cpp
module;
template <typename T> struct S;
export module M;
S(int) -> S<double>;
// x.cpp
template <typename T> struct S { S(int); };
import M;
int main() {
S s(10); // should be S<double> s;
}
This patch doesn't correctly handle this case yet, we need to also
consider cases where only the deduction guide is in purview.
Indeed.
Jason
Here's a small update to the patch which fixes the above bug by ensuring
that dependencies are created for purview deduction guides rather than
just being skipped entirely, and includes that testcase with dguide-3.
This patch is still using force-exported bindings rather than attempting
to do anything more clever with name lookup just yet.
Bootstrapped and regtested (so far just modules.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
-- >8 --
Deduction guides are represented as 'normal' functions currently, and
have no special handling in modules. However, this causes some issues;
by [temp.deduct.guide] a deduction guide is not found by normal name
lookup and instead all reachable deduction guides for a class template
should be considered, but this does not happen currently.
To solve this, this patch ensures that all deduction guides are
considered exported to ensure that they are always visible to importers
if they are reachable. Another alternative here would be to add a new
kind of "all reachable" flag to name lookup, but that is complicated by
some difficulties in handling GM entities; this may be a better way to
go if more kinds of entities end up needing this handling, however.
Another issue here is that because deduction guides are "unrelated"
functions, they will usually get discarded from the GMF, so this patch
ensures that when finding dependencies, GMF deduction guides will also
have bindings created. We do this in find_dependencies so that we don't
unnecessarily create bindings for GMF deduction guides that are never
reached; for consistency we do this for *all* deduction guides, not just
GM ones. We also make sure that the opposite (a deduction guide being
the only purview reference to a GMF template) correctly marks it as
reachable.
Finally, when merging deduction guides from multiple modules, the name
lookup code may now return two-dimensional overload sets, so update
callers to match.
As a small drive-by improvement this patch also updates the error pretty
printing code to add a space before the '->' when printing a deduction
guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.
@@ -15158,6 +15218,10 @@ module_state::write_cluster (elf_out *to, depset
*scc[], unsigned size,
flags |= cbf_hidden;
else if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (bound)))
flags |= cbf_export;
+ else if (deduction_guide_p (bound))
+ /* Deduction guides are always exported so that they are
+ reachable whenever their class template is. */
I'd adjust "reachable" to "visible to name lookup"; they don't need to
be exported to be reachable. OK with that change.
Jason
