Skip to content

Commit 9f9afc6

Browse files
committed
c++/modules: Handle forward-declared class types
In some cases we can access members of a namespace-scope class without ever having performed name-lookup on it; this can occur when a forward-declaration of the class is used as a return type, for instance, or with PIMPL. One possible approach would be to do name lookup in complete_type to force lazy loading to occur, but this seems overly expensive for a relatively rare case. Instead, this patch generalises the existing pending-entity support to handle this case as well. Unfortunately this does mean that almost every class definition will be added to the pending-entity table, and almost always unnecessarily, but I don't see a good way to avoid this. gcc/cp/ChangeLog: * module.cc (depset::DB_IS_MEMBER_BIT): Rename to... (depset::DB_IS_PENDING_BIT): ...this. (depset::is_member): Remove. (depset::is_pending_entity): New function. (depset::hash::make_dependency): Mark definitions of namespace-scope types as maybe-pending entities. (depset::hash::add_class_entities): Rename DB_IS_MEMBER_BIT to DB_IS_PENDING_BIT. (depset::hash::find_dependencies): Use is_pending_entity instead of is_member. (module_state::write_pendings): Likewise; adjust comment. gcc/testsuite/ChangeLog: * g++.dg/modules/inst-4_b.C: Adjust pending-entity count. * g++.dg/modules/member-def-1_c.C: Likewise. * g++.dg/modules/member-def-2_c.C: Likewise. * g++.dg/modules/tpl-spec-3_b.C: Likewise. * g++.dg/modules/tpl-spec-4_b.C: Likewise. * g++.dg/modules/tpl-spec-5_b.C: Likewise. * g++.dg/modules/class-9_a.H: New test. * g++.dg/modules/class-9_b.H: New test. * g++.dg/modules/class-9_c.C: New test. Signed-off-by: Nathaniel Shead <[email protected]> Reviewed-by: Jason Merrill <[email protected]>
1 parent d464a52 commit 9f9afc6

File tree

10 files changed

+64
-30
lines changed

10 files changed

+64
-30
lines changed

gcc/cp/module.cc

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2329,7 +2329,7 @@ class depset {
23292329
DB_KIND_BIT, /* Kind of the entity. */
23302330
DB_KIND_BITS = EK_BITS,
23312331
DB_DEFN_BIT = DB_KIND_BIT + DB_KIND_BITS,
2332-
DB_IS_MEMBER_BIT, /* Is an out-of-class member. */
2332+
DB_IS_PENDING_BIT, /* Is a maybe-pending entity. */
23332333
DB_IS_INTERNAL_BIT, /* It is an (erroneous)
23342334
internal-linkage entity. */
23352335
DB_REFS_INTERNAL_BIT, /* Refers to an internal-linkage
@@ -2407,11 +2407,14 @@ class depset {
24072407
}
24082408

24092409
public:
2410-
/* This class-member is defined here, but the class was imported. */
2411-
bool is_member () const
2410+
/* This entity might be found other than by namespace-scope lookup;
2411+
see module_state::write_pendings for more details. */
2412+
bool is_pending_entity () const
24122413
{
2413-
gcc_checking_assert (get_entity_kind () == EK_DECL);
2414-
return get_flag_bit<DB_IS_MEMBER_BIT> ();
2414+
return (get_entity_kind () == EK_SPECIALIZATION
2415+
|| get_entity_kind () == EK_PARTIAL
2416+
|| (get_entity_kind () == EK_DECL
2417+
&& get_flag_bit<DB_IS_PENDING_BIT> ()));
24152418
}
24162419
public:
24172420
bool is_internal () const
@@ -13031,6 +13034,18 @@ depset::hash::make_dependency (tree decl, entity_kind ek)
1303113034
dep->set_flag_bit<DB_IS_INTERNAL_BIT> ();
1303213035
}
1303313036
}
13037+
13038+
/* A namespace-scope type may be declared in one module unit
13039+
and defined in another; make sure that we're found when
13040+
completing the class. */
13041+
if (ek == EK_DECL
13042+
&& !dep->is_import ()
13043+
&& dep->has_defn ()
13044+
&& DECL_NAMESPACE_SCOPE_P (not_tmpl)
13045+
&& DECL_IMPLICIT_TYPEDEF_P (not_tmpl)
13046+
/* Anonymous types can't be forward-declared. */
13047+
&& !IDENTIFIER_ANON_P (DECL_NAME (not_tmpl)))
13048+
dep->set_flag_bit<DB_IS_PENDING_BIT> ();
1303413049
}
1303513050

1303613051
if (!dep->is_import ())
@@ -13383,9 +13398,9 @@ depset::hash::add_class_entities (vec<tree, va_gc> *class_members)
1338313398
if (dep->get_entity_kind () == EK_REDIRECT)
1338413399
dep = dep->deps[0];
1338513400

13386-
/* Only non-instantiations need marking as members. */
13401+
/* Only non-instantiations need marking as pendings. */
1338713402
if (dep->get_entity_kind () == EK_DECL)
13388-
dep->set_flag_bit <DB_IS_MEMBER_BIT> ();
13403+
dep->set_flag_bit <DB_IS_PENDING_BIT> ();
1338913404
}
1339013405
}
1339113406

@@ -13711,10 +13726,7 @@ depset::hash::find_dependencies (module_state *module)
1371113726
walker.mark_declaration (decl, current->has_defn ());
1371213727

1371313728
if (!walker.is_key_order ()
13714-
&& (item->get_entity_kind () == EK_SPECIALIZATION
13715-
|| item->get_entity_kind () == EK_PARTIAL
13716-
|| (item->get_entity_kind () == EK_DECL
13717-
&& item->is_member ())))
13729+
&& item->is_pending_entity ())
1371813730
{
1371913731
tree ns = find_pending_key (decl, nullptr);
1372013732
add_namespace_context (item, ns);
@@ -15939,15 +15951,13 @@ module_state::read_entities (unsigned count, unsigned lwm, unsigned hwm)
1593915951
'instantiated' in one module, and it'd be nice to not have to
1594015952
reinstantiate it in another.
1594115953

15942-
(c) A member classes completed elsewhere. A member class could be
15943-
declared in one header and defined in another. We need to know to
15944-
load the class definition before looking in it. This turns out to
15945-
be a specific case of #b, so we can treat these the same. But it
15946-
does highlight an issue -- there could be an intermediate import
15947-
between the outermost containing namespace-scope class and the
15948-
innermost being-defined member class. This is actually possible
15949-
with all of these cases, so be aware -- we're not just talking of
15950-
one level of import to get to the innermost namespace.
15954+
(c) Classes completed elsewhere. A class could be declared in one
15955+
header and defined in another. We need to know to load the class
15956+
definition before looking in it. It does highlight an issue --
15957+
there could be an intermediate import between the outermost containing
15958+
namespace-scope class and the innermost being-defined class. This is
15959+
actually possible with all of these cases, so be aware -- we're not
15960+
just talking of one level of import to get to the innermost namespace.
1595115961

1595215962
This gets complicated fast, it took me multiple attempts to even
1595315963
get something remotely working. Partially because I focussed on
@@ -16067,9 +16077,7 @@ module_state::write_pendings (elf_out *to, vec<depset *> depsets,
1606716077
if (d->is_import ())
1606816078
continue;
1606916079

16070-
if (!(d->get_entity_kind () == depset::EK_SPECIALIZATION
16071-
|| d->get_entity_kind () == depset::EK_PARTIAL
16072-
|| (d->get_entity_kind () == depset::EK_DECL && d->is_member ())))
16080+
if (!d->is_pending_entity ())
1607316081
continue;
1607416082

1607516083
tree key_decl = nullptr;
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// { dg-additional-options "-fmodule-header" }
2+
// { dg-module-cmi {} }
3+
4+
struct A;
5+
A* foo();
6+
7+
template <typename T> struct B;
8+
template <typename T> B<T>* bar();
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// { dg-additional-options "-fmodule-header -fdump-lang-module" }
2+
// { dg-module-cmi {} }
3+
4+
struct A { int a; };
5+
template <typename T> struct B { int b; };
6+
7+
// { dg-final { scan-lang-dump {Pendings 2} module } }
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// { dg-additional-options "-fmodules-ts -fmodule-lazy" }
2+
3+
import "class-9_a.H";
4+
import "class-9_b.H";
5+
6+
int main() {
7+
// Lazy loading should still find the definitions of A and B.
8+
int a = foo()->a;
9+
int b = bar<int>()->b;
10+
}

gcc/testsuite/g++.dg/modules/inst-4_b.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ int main ()
99
return 0;
1010
}
1111

12-
// { dg-final { scan-lang-dump {Reading 1 pending entities keyed to '::TPL'} module } }
12+
// { dg-final { scan-lang-dump {Reading 2 pending entities keyed to '::TPL'} module } }
1313
// { dg-final { scan-lang-dump {Read:-[0-9]*'s type spec merge key \(new\) type_decl:'::TPL'} module } }

gcc/testsuite/g++.dg/modules/member-def-1_c.C

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ export auto foo ()
1111
return frob::inner ();
1212
}
1313

14-
// { dg-final { scan-lang-dump {Reading 1 pending entities keyed to '::frob'} module } }
14+
// { dg-final { scan-lang-dump {Reading 2 pending entities keyed to '::frob'} module } }
1515
// { dg-final { scan-lang-dump { Cluster members:\n \[0\]=decl definition '::frob@foo:part1:1'\n \[1\]=decl definition '::frob@foo:part1:1::inner@foo:part1:1'\n \[2\]=decl declaration '::frob@foo:part1:1::inner@foo:part1:1::__dt '\n( \[.\]=decl declaration '::frob@foo:part1:1::inner@foo:part1:1::__ct '\n)* \[6\]=decl declaration '::frob@foo:part1:1::inner@foo:part1:1::inner@foo:part2:2'\n \[7\]=decl declaration '::frob@foo:part1:1::frob@foo:part1:1'\n \[8\]=decl declaration '::frob@foo:part1:1::__as_base @foo:part1:1'\n \[9\]=binding '::frob'\n} module } }
16-
// { dg-final { scan-lang-dump {Pendings 0} module } }
16+
// { dg-final { scan-lang-dump {Pendings 1} module } }

gcc/testsuite/g++.dg/modules/member-def-2_c.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export import :part1;
99

1010
// { dg-final { scan-lang-dump { Cluster members:\n \[0\]=decl definition '::frob@foo:part1:1'\n \[1\]=decl declaration '::frob@foo:part1:1::frob@foo:part1:1'\n \[2\]=decl definition '::frob@foo:part1:1::member@foo:part1:1'\n \[3\]=decl declaration '::frob@foo:part1:1::__as_base @foo:part1:1'\n \[4\]=binding '::frob'\n} module } }
1111
// { dg-final { scan-lang-dump {Bindings 1} module } }
12-
// { dg-final { scan-lang-dump {Pendings 0} module } }
12+
// { dg-final { scan-lang-dump {Pendings 1} module } }
1313
// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key .matched. function_decl:'::frob@foo:part1:1::member'} module } }
1414

1515
// { dg-final { scan-assembler-not {_ZN4frob6memberEv:} } }

gcc/testsuite/g++.dg/modules/tpl-spec-3_b.C

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ int main ()
1717
return 0;
1818
}
1919

20-
// { dg-final { scan-lang-dump {Reading 1 pending entities keyed to '::frob'} module } }
20+
// We read a pending for both '::frob' and '::frob::store'.
21+
// { dg-final { scan-lang-dump {Reading 2 pending entities keyed to '::frob'} module } }
2122
// { dg-final { scan-lang-dump-not {Reading definition function_decl '::frob@TPL:.::store@TPL:.<int>'} module } }
2223

2324
// { dg-final { scan-assembler-not {_ZN4frob5storeIiEEvT_:} } }

gcc/testsuite/g++.dg/modules/tpl-spec-4_b.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ int main ()
1414
return 0;
1515
}
1616

17-
// { dg-final { scan-lang-dump {Reading 1 pending entities keyed to '::X'} module } }
17+
// { dg-final { scan-lang-dump {Reading 2 pending entities keyed to '::X'} module } }

gcc/testsuite/g++.dg/modules/tpl-spec-5_b.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ int main ()
1414
return 0;
1515
}
1616

17-
// { dg-final { scan-lang-dump {Reading 1 pending entities keyed to '::X'} module } }
17+
// { dg-final { scan-lang-dump {Reading 2 pending entities keyed to '::X'} module } }

0 commit comments

Comments
 (0)