Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
Or would you prefer a different approach?
-- >8 --
The testcase shows an issue where we end up indirectly loading a
special member function while we're attempting to lazily declare it.
This causes errors in the install_implicit_member because
CLASSTYPE_LAZY* has been cleared, so it assumes that the member exists
already.
There are a few options to approach this issue:
1. Remove the error when we fail with install_implicit_member, and just
silently assume that a case like this is happening. This should be
safe, but could make it harder to understand future issues in this
area.
2. Put all implicitly declared special members on the pending list for
the class type, and add calls to lazy_load_pendings at the start of
lazily_declare_fn and check if the functions have now been
provided. But this could unnecessarily bloat out the pending tables
as aggregate types used in lots of modules will add their special
members to every module's pending table.
3. Detect exactly this special case in implicitly_declare_fn and halt
processing. This should only impact lazily_declare_fn, but makes its
API more complex.
This patch takes option three. Just because we know that the function
has been lazily declared somewhere else doesn't mean we can easily find
its declaration, however; luckily nothing actually uses the return type
of lazily_declare_fn, so we can change that to void instead and save on
some effort.
PR c++/124478
gcc/cp/ChangeLog:
* cp-tree.h (lazily_declare_fn): Make return type void.
* method.cc (is_lazy_special_member): New function.
(implicitly_declare_fn): Bail early if we lazy loaded the member
we're trying to declare.
(lazily_declare_fn): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/imp-member-6_a.C: New test.
* g++.dg/modules/imp-member-6_b.C: New test.
* g++.dg/modules/imp-member-6_c.C: New test.
Signed-off-by: Nathaniel Shead <[email protected]>
---
gcc/cp/cp-tree.h | 2 +-
gcc/cp/method.cc | 66 +++++++++++++++----
gcc/testsuite/g++.dg/modules/imp-member-6_a.C | 15 +++++
gcc/testsuite/g++.dg/modules/imp-member-6_b.C | 9 +++
gcc/testsuite/g++.dg/modules/imp-member-6_c.C | 11 ++++
5 files changed, 88 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/imp-member-6_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/imp-member-6_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/imp-member-6_c.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 7f31aca47b0..7444cc83770 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7879,7 +7879,7 @@ extern bool deduce_inheriting_ctor (tree);
extern bool decl_remember_implicit_trigger_p (tree);
extern void synthesize_method (tree);
extern void maybe_synthesize_method (tree);
-extern tree lazily_declare_fn (special_function_kind,
+extern void lazily_declare_fn (special_function_kind,
tree);
extern tree skip_artificial_parms_for (const_tree, tree);
extern int num_artificial_parms_for (const_tree);
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 94fdc32d0bd..fe26b5130ff 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -3356,11 +3356,38 @@ deduce_inheriting_ctor (tree decl)
return true;
}
+/* Returns whether SFK is currently lazy within TYPE, i.e., it hasn't
+ yet been declared. */
+
+static bool
+is_lazy_special_member (special_function_kind sfk, tree type)
+{
+ switch (sfk)
+ {
+ case sfk_constructor:
+ return CLASSTYPE_LAZY_DEFAULT_CTOR (type);
+ case sfk_copy_constructor:
+ return CLASSTYPE_LAZY_COPY_CTOR (type);
+ case sfk_move_constructor:
+ return CLASSTYPE_LAZY_MOVE_CTOR (type);
+ case sfk_copy_assignment:
+ return CLASSTYPE_LAZY_COPY_ASSIGN (type);
+ case sfk_move_assignment:
+ return CLASSTYPE_LAZY_MOVE_ASSIGN (type);
+ case sfk_destructor:
+ return CLASSTYPE_LAZY_DESTRUCTOR (type);
+ default:
+ return false;
+ }
+}
+
/* Implicitly declare the special function indicated by KIND, as a
member of TYPE. For copy constructors and assignment operators,
CONST_P indicates whether these functions should take a const
reference argument or a non-const reference.
- Returns the FUNCTION_DECL for the implicitly declared function. */
+ Returns the FUNCTION_DECL for the new implicitly declared function,
+ or NULL_TREE if we just discovered the function already exists.
+ Currently this can only happen for lazy implicit members. */
tree
implicitly_declare_fn (special_function_kind kind, tree type,
@@ -3484,6 +3511,7 @@ implicitly_declare_fn (special_function_kind kind, tree
type,
}
bool trivial_p = false;
+ bool was_lazy = is_lazy_special_member (kind, type);
if (inherited_ctor)
{
@@ -3504,6 +3532,14 @@ implicitly_declare_fn (special_function_kind kind, tree
type,
synthesized_method_walk (type, kind, const_p, &raises, &trivial_p,
&deleted_p, &constexpr_p, false,
&inherited_ctor, inherited_parms);
+
+ /* The above walk may have indirectly loaded a lazy decl we're
+ about to build from a module, let's not build it again. */
+ if (modules_p ()
+ && was_lazy
+ && !is_lazy_special_member (kind, type))
+ return NULL_TREE;
+
/* Don't bother marking a deleted constructor as constexpr. */
if (deleted_p)
constexpr_p = false;
@@ -3918,17 +3954,26 @@ defaultable_fn_check (tree fn)
}
/* Add an implicit declaration to TYPE for the kind of function
- indicated by SFK. Return the FUNCTION_DECL for the new implicit
- declaration. */
+ indicated by SFK. */
-tree
+void
lazily_declare_fn (special_function_kind sfk, tree type)
{
- tree fn;
+ type = TYPE_MAIN_VARIANT (type);
+
/* Whether or not the argument has a const reference type. */
- bool const_p = false;
+ bool const_p = ((sfk == sfk_copy_constructor
+ && TYPE_HAS_CONST_COPY_CTOR (type))
+ || (sfk == sfk_copy_assignment
+ && TYPE_HAS_CONST_COPY_ASSIGN (type)));
- type = TYPE_MAIN_VARIANT (type);
+ /* Declare the function. */
+ tree fn = implicitly_declare_fn (sfk, type, const_p, NULL, NULL);
+
+ /* We may have indirectly acquired the function from a module,
+ if so there's nothing else to do. */
+ if (!fn)
+ return;
switch (sfk)
{
@@ -3936,14 +3981,12 @@ lazily_declare_fn (special_function_kind sfk, tree type)
CLASSTYPE_LAZY_DEFAULT_CTOR (type) = 0;
break;
case sfk_copy_constructor:
- const_p = TYPE_HAS_CONST_COPY_CTOR (type);
CLASSTYPE_LAZY_COPY_CTOR (type) = 0;
break;
case sfk_move_constructor:
CLASSTYPE_LAZY_MOVE_CTOR (type) = 0;
break;
case sfk_copy_assignment:
- const_p = TYPE_HAS_CONST_COPY_ASSIGN (type);
CLASSTYPE_LAZY_COPY_ASSIGN (type) = 0;
break;
case sfk_move_assignment:
@@ -3956,9 +3999,6 @@ lazily_declare_fn (special_function_kind sfk, tree type)
gcc_unreachable ();
}
- /* Declare the function. */
- fn = implicitly_declare_fn (sfk, type, const_p, NULL, NULL);
-
/* [class.copy]/8 If the class definition declares a move constructor or
move assignment operator, the implicitly declared copy constructor is
defined as deleted.... */
@@ -4011,8 +4051,6 @@ lazily_declare_fn (special_function_kind sfk, tree type)
check_bases_and_members, but we must also inject them here for deferred
lazily-declared functions. */
maybe_propagate_warmth_attributes (fn, type);
-
- return fn;
}
/* Given a FUNCTION_DECL FN and a chain LIST, skip as many elements of LIST
diff --git a/gcc/testsuite/g++.dg/modules/imp-member-6_a.C
b/gcc/testsuite/g++.dg/modules/imp-member-6_a.C
new file mode 100644
index 00000000000..25c4838d4c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/imp-member-6_a.C
@@ -0,0 +1,15 @@
+// PR c++/124478
+// { dg-additional-options "-fmodules -fdump-lang-module" }
+// { dg-module-cmi tools }
+
+export module tools;
+export template <typename T> struct myvector {
+ T* t = nullptr;
+ ~myvector() { delete t; }
+};
+export struct DedupeFilesPath {
+ myvector<int> other;
+};
+
+// Lazy destructor
+// { dg-final { scan-lang-dump-not {'::DedupeFilesPath@tools:.::__dt '} module
} }
diff --git a/gcc/testsuite/g++.dg/modules/imp-member-6_b.C
b/gcc/testsuite/g++.dg/modules/imp-member-6_b.C
new file mode 100644
index 00000000000..06c34f134f5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/imp-member-6_b.C
@@ -0,0 +1,9 @@
+// PR c++/124478
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi dedupe }
+
+export module dedupe;
+import tools;
+
+// This requires the destructor to exist, and is placed in CMI.
+myvector<DedupeFilesPath> vec;
diff --git a/gcc/testsuite/g++.dg/modules/imp-member-6_c.C
b/gcc/testsuite/g++.dg/modules/imp-member-6_c.C
new file mode 100644
index 00000000000..dee04c6d3fe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/imp-member-6_c.C
@@ -0,0 +1,11 @@
+// PR c++/124478
+// { dg-additional-options "-fmodules -fdump-lang-module-alias" }
+
+import tools;
+import dedupe;
+DedupeFilesPath analyse(int) {
+ return {};
+}
+
+// Find the implicit member from dedupe when attempting to instantiate it
ourselves.
+// { dg-final { scan-lang-dump {Adding implicit member
'::DedupeFilesPath@tools:.::__dt @dedupe:.'} module } }
--
2.51.0
