On Thu, Apr 10, 2025 at 12:19:57PM -0400, Jason Merrill wrote:
> On 4/10/25 8:46 AM, Nathaniel Shead wrote:
> > Regression raised with my by private correspondance.
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > -- >8 --
> >
> > My change in r15-9216 broke the case where we imported an uninstantiated
> > defaulted function over the top of one we had already finished. This
> > patch ensures that we don't error for mismatches when either function
> > has mismatching deferral from the other.
>
> These changes seem to mean that importing a deferred decl can overwrite
> properties of an existing non-deferred one.
>
Right.
> > To assist in understanding these errors in the future as well, I've
> > customised the mismatch message for different cases. I also rephrased
> > the message to talk about "imported declarations" rather than "global
> > module declarations", since as the FIXME noted we can also get
> > mismatches with some declarations attached to modules. Ideally I'd like
> > to revisit the way this is structured entirely but that won't be
> > appropriate for GCC 15.
>
> Please split these changes into a separate commit; that patch is OK.
>
Thanks, I've attached the patch I'll push after regtest/bootstrap.
The following patch tested so far on x86_64-pc-linux-gnu (only
modules.exp), ok for trunk if full bootstrap+regtest succeeds?
-- >8 --
My change in r15-9216 broke the case where we imported an uninstantiated
defaulted function over the top of one we had already finished. This
patch ensures that we don't error for mismatches in this case.
gcc/cp/ChangeLog:
* module.cc (trees_in::is_matching_decl): Don't check for
mismatches when importing a DECL_MAYBE_DELETED function over one
that's already finished.
gcc/testsuite/ChangeLog:
* g++.dg/modules/noexcept-4_a.H: New test.
* g++.dg/modules/noexcept-4_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/module.cc | 5 ++++-
gcc/testsuite/g++.dg/modules/noexcept-4_a.H | 6 ++++++
gcc/testsuite/g++.dg/modules/noexcept-4_b.C | 18 ++++++++++++++++++
3 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-4_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-4_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 8efa18baff1..5ff5c462e79 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12164,7 +12164,8 @@ trees_in::is_matching_decl (tree existing, tree decl,
bool is_typedef)
}
}
}
- else if (!DEFERRED_NOEXCEPT_SPEC_P (d_spec)
+ else if (!DECL_MAYBE_DELETED (d_inner)
+ && !DEFERRED_NOEXCEPT_SPEC_P (d_spec)
&& !comp_except_specs (d_spec, e_spec, ce_type))
{
mismatch_msg = G_("conflicting %<noexcept%> specifier for "
@@ -12195,6 +12196,8 @@ trees_in::is_matching_decl (tree existing, tree decl,
bool is_typedef)
if (DECL_MAYBE_DELETED (e_inner) && !DECL_MAYBE_DELETED (d_inner)
&& DECL_DECLARED_CONSTEXPR_P (d_inner))
DECL_DECLARED_CONSTEXPR_P (e_inner) = true;
+ else if (!DECL_MAYBE_DELETED (e_inner) && DECL_MAYBE_DELETED (d_inner))
+ /* Nothing to do. */;
else if (DECL_DECLARED_CONSTEXPR_P (e_inner)
!= DECL_DECLARED_CONSTEXPR_P (d_inner))
{
diff --git a/gcc/testsuite/g++.dg/modules/noexcept-4_a.H
b/gcc/testsuite/g++.dg/modules/noexcept-4_a.H
new file mode 100644
index 00000000000..b888a1b93e6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/noexcept-4_a.H
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodule-header -std=c++20" }
+// { dg-module-cmi {} }
+
+struct exception_ptr {
+ friend bool operator==(const exception_ptr&, const exception_ptr&) = default;
+};
diff --git a/gcc/testsuite/g++.dg/modules/noexcept-4_b.C
b/gcc/testsuite/g++.dg/modules/noexcept-4_b.C
new file mode 100644
index 00000000000..7cc55316c7c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/noexcept-4_b.C
@@ -0,0 +1,18 @@
+// { dg-additional-options "-fmodules -std=c++20" }
+
+struct exception_ptr {
+ friend bool operator==(const exception_ptr&, const exception_ptr&) = default;
+};
+
+void enqueue() {
+ exception_ptr e;
+ e == e;
+}
+
+import "noexcept-4_a.H";
+
+int main() {
+ constexpr exception_ptr e;
+ static_assert(e == e);
+ static_assert(noexcept(e == e));
+}
--
2.47.0
>From 95a4827b96efa193eda2032e28ad8133373e884d Mon Sep 17 00:00:00 2001
From: Nathaniel Shead <nathanielosh...@gmail.com>
Date: Fri, 11 Apr 2025 07:09:33 +1000
Subject: [PATCH 1/2] c++/modules: Give more specific diagnostics in
is_matching_decl
This patch also rephrases the diagnostics to talk about "imported
declarations" rather than "global module declarations", since as the
FIXME noted we can also get mismatches with some declarations attached
to modules. Ideally I'd like to revisit the way this is structured
entirely but that won't be appropriate for GCC 15.
gcc/cp/ChangeLog:
* module.cc (trees_in::is_matching_decl): Add custom errors for
different kinds of mismatches.
gcc/testsuite/ChangeLog:
* g++.dg/modules/lambda-8_b.C: Adjust error.
* g++.dg/modules/leg-merge-4_c.C: Likewise.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/module.cc | 55 +++++++++++++++-----
gcc/testsuite/g++.dg/modules/lambda-8_b.C | 2 +-
gcc/testsuite/g++.dg/modules/leg-merge-4_c.C | 6 +--
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 37fab5b5a43..8efa18baff1 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12090,6 +12090,8 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
gcc_checking_assert (TREE_CODE (e_inner) == TREE_CODE (d_inner));
}
+ // FIXME: do more precise errors at point of mismatch
+ const char *mismatch_msg = nullptr;
if (TREE_CODE (d_inner) == FUNCTION_DECL)
{
tree e_ret = fndecl_declared_return_type (existing);
@@ -12099,13 +12101,20 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
&& LAMBDA_TYPE_P (DECL_CONTEXT (d_inner)))
/* This has a recursive type that will compare different. */;
else if (!same_type_p (d_ret, e_ret))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting type for imported declaration %#qD");
+ goto mismatch;
+ }
tree e_type = TREE_TYPE (e_inner);
tree d_type = TREE_TYPE (d_inner);
if (DECL_EXTERN_C_P (d_inner) != DECL_EXTERN_C_P (e_inner))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting language linkage for imported "
+ "declaration %#qD");
+ goto mismatch;
+ }
for (tree e_args = TYPE_ARG_TYPES (e_type),
d_args = TYPE_ARG_TYPES (d_type);
@@ -12113,10 +12122,18 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
e_args = TREE_CHAIN (e_args), d_args = TREE_CHAIN (d_args))
{
if (!(e_args && d_args))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting argument list for imported "
+ "declaration %#qD");
+ goto mismatch;
+ }
if (!same_type_p (TREE_VALUE (d_args), TREE_VALUE (e_args)))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting argument types for imported "
+ "declaration %#qD");
+ goto mismatch;
+ }
}
/* If EXISTING has an undeduced or uninstantiated exception
@@ -12149,7 +12166,11 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
}
else if (!DEFERRED_NOEXCEPT_SPEC_P (d_spec)
&& !comp_except_specs (d_spec, e_spec, ce_type))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting %<noexcept%> specifier for "
+ "imported declaration %#qD");
+ goto mismatch;
+ }
/* Similarly if EXISTING has an undeduced return type, but DECL's
is already deduced. */
@@ -12163,7 +12184,11 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
}
else if (type_uses_auto (d_ret)
&& !same_type_p (TREE_TYPE (d_type), TREE_TYPE (e_type)))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting deduced return type for "
+ "imported declaration %#qD");
+ goto mismatch;
+ }
/* Similarly if EXISTING has undeduced constexpr, but DECL's
is already deduced. */
@@ -12172,7 +12197,11 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
DECL_DECLARED_CONSTEXPR_P (e_inner) = true;
else if (DECL_DECLARED_CONSTEXPR_P (e_inner)
!= DECL_DECLARED_CONSTEXPR_P (d_inner))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting %<constexpr%> for imported "
+ "declaration %#qD");
+ goto mismatch;
+ }
/* Don't synthesize a defaulted function if we're importing one
we've already determined. */
@@ -12184,13 +12213,17 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
if (!DECL_ORIGINAL_TYPE (e_inner)
|| !same_type_p (DECL_ORIGINAL_TYPE (d_inner),
DECL_ORIGINAL_TYPE (e_inner)))
- goto mismatch;
+ {
+ mismatch_msg = G_("conflicting imported declaration %q#D");
+ goto mismatch;
+ }
}
/* Using cp_tree_equal because we can meet TYPE_ARGUMENT_PACKs
here. I suspect the entities that directly do that are things
that shouldn't go to duplicate_decls (FIELD_DECLs etc). */
else if (!cp_tree_equal (TREE_TYPE (decl), TREE_TYPE (existing)))
{
+ mismatch_msg = G_("conflicting type for imported declaration %#qD");
mismatch:
if (DECL_IS_UNDECLARED_BUILTIN (existing))
/* Just like duplicate_decls, presum the user knows what
@@ -12203,11 +12236,9 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
equality isn't feasible in general for local entities. */;
else
{
- // FIXME:QOI Might be template specialization from a module,
- // not necessarily global module
+ gcc_checking_assert (mismatch_msg);
auto_diagnostic_group d;
- error_at (DECL_SOURCE_LOCATION (decl),
- "conflicting global module declaration %#qD", decl);
+ error_at (DECL_SOURCE_LOCATION (decl), mismatch_msg, decl);
inform (DECL_SOURCE_LOCATION (existing),
"existing declaration %#qD", existing);
return false;
diff --git a/gcc/testsuite/g++.dg/modules/lambda-8_b.C b/gcc/testsuite/g++.dg/modules/lambda-8_b.C
index 7ace4944dd2..96578ba94cb 100644
--- a/gcc/testsuite/g++.dg/modules/lambda-8_b.C
+++ b/gcc/testsuite/g++.dg/modules/lambda-8_b.C
@@ -4,4 +4,4 @@
#include "lambda-8.h"
import "lambda-8_a.H";
-// { dg-error "conflicting global module declaration" "" { target *-*-* } 0 }
+// { dg-error "conflicting imported declaration" "" { target *-*-* } 0 }
diff --git a/gcc/testsuite/g++.dg/modules/leg-merge-4_c.C b/gcc/testsuite/g++.dg/modules/leg-merge-4_c.C
index f1b1aeb5b37..5756057ce0b 100644
--- a/gcc/testsuite/g++.dg/modules/leg-merge-4_c.C
+++ b/gcc/testsuite/g++.dg/modules/leg-merge-4_c.C
@@ -11,8 +11,8 @@ void foo ()
X *p;
}
-// { dg-regexp "\nIn module \[^\n]*leg-merge-4_b.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_b.H:4:\[0-9]*: error: conflicting global module declaration 'float bob'\nIn module \[^\n]*leg-merge-4_a.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_a.H:4:\[0-9]*: note: existing declaration 'int bob'\n\[^\n]*leg-merge-4_c.C:9:\[0-9]*: note: during load of binding '::bob'$" }
+// { dg-regexp "\nIn module \[^\n]*leg-merge-4_b.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_b.H:4:\[0-9]*: error: conflicting type for imported declaration 'float bob'\nIn module \[^\n]*leg-merge-4_a.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_a.H:4:\[0-9]*: note: existing declaration 'int bob'\n\[^\n]*leg-merge-4_c.C:9:\[0-9]*: note: during load of binding '::bob'$" }
-// { dg-regexp "\nIn module \[^\n]*leg-merge-4_b.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_b.H:5:\[0-9]*: error: conflicting global module declaration 'int frob\\(\\)'\nIn module \[^\n]*leg-merge-4_a.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_a.H:5:\[0-9]*: note: existing declaration 'void frob\\(\\)'\n\[^\n]*leg-merge-4_c.C:10:\[0-9]*: note: during load of binding '::frob'$" }
+// { dg-regexp "\nIn module \[^\n]*leg-merge-4_b.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_b.H:5:\[0-9]*: error: conflicting type for imported declaration 'int frob\\(\\)'\nIn module \[^\n]*leg-merge-4_a.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_a.H:5:\[0-9]*: note: existing declaration 'void frob\\(\\)'\n\[^\n]*leg-merge-4_c.C:10:\[0-9]*: note: during load of binding '::frob'$" }
-// { dg-regexp "In module \[^\n]*leg-merge-4_b.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_b.H:6:\[0-9]*: error: conflicting global module declaration 'union X'\nIn module \[^\n]*leg-merge-4_a.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_a.H:6:\[0-9]*: note: existing declaration 'class X'\n\[^\n]*leg-merge-4_c.C:11:\[0-9]*: note: during load of binding '::X'$" }
+// { dg-regexp "In module \[^\n]*leg-merge-4_b.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_b.H:6:\[0-9]*: error: conflicting type for imported declaration 'union X'\nIn module \[^\n]*leg-merge-4_a.H, imported at \[^\n]*leg-merge-4_c.C:\[0-9]*:\n\[^\n]*leg-merge-4_a.H:6:\[0-9]*: note: existing declaration 'class X'\n\[^\n]*leg-merge-4_c.C:11:\[0-9]*: note: during load of binding '::X'$" }
--
2.47.0
