On 1/12/24 09:09, Jason Merrill wrote:
On 12/23/23 14:46, Nathan Sidwell wrote:
On 12/18/23 17:10, Jason Merrill wrote:
On 12/18/23 16:57, Nathan Sidwell wrote:
On 12/18/23 16:31, Jason Merrill wrote:
Tested x86_64-pc-linux-gnu. Does this make sense? Did you have
another theory
about how to merge these?
Why isn't push_abi_namespace doing the right setup here? (and I
think get_global_binding might be similarly problematic?)
What would the right setup be? It pushes into the global module, but
before this change lookup doesn't find things imported into the
global module, and so we get two independent (and so non-equivalent)
declarations.
The comment for get_namespace_binding says "Users of this who, having
found nothing, push a new decl must be prepared for that pushing to
match an existing decl." But if lookup_elaborated_type fails, so we
pushtag a new type, check_module_override doesn't try to merge them
because TREE_PUBLIC isn't set on the TYPE_DECL yet at that point, and
they coexist until we complain about redeclaring __dynamic_cast with
non-matching parameter types.
I tried setting TREE_PUBLIC on the TYPE_DECL, and then
check_module_override called duplicate_decls, and rejected the
redeclaration as a different type.
sigh, it seems that doesn't work as intended, I guess your approace is
a pragmatic workaround, much as I dislike special-casing particular
identifier. Perhaps comment with an appropriate FIXME?
I've realized there's problems with completeness here -- the
'invisible' type may be complete, but the current TU only
forward-declares it. Our AST can't represent that right now. And I'm
not sure if there are template instantiation issues -- is the type
complete or not in any particular instantiaton?
My understanding of https://eel.is/c++draft/module#reach-4 is that this
doesn't matter: if there is a reachable definition of the class, the
class is complete, even if the current TU only forward-declares it.
Here's an alternate approach that handles this merging in
check_module_override; this makes P1811 include-after-import a bit
worse, but it's already not well supported, so perhaps that's OK for
now. But I'm inclined to go with my earlier patch for GCC 14. What do
you think?
I'm going to go ahead and push this revision of my earlier patch for
now, we can adjust as needed.
From 27521a2f4f7b859d5656e5bdd69d3f759ea4c23a Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Mon, 18 Dec 2023 15:47:10 -0500
Subject: [PATCH] c++: __class_type_info and modules [PR113038]
To: gcc-patches@gcc.gnu.org
Doing a dynamic_cast in both TUs broke because we were declaring a new
__class_type_info in _b that conflicted with the one imported in the global
module from _a. It seems clear to me that any new class declaration in
the global module should merge with an imported definition, but for GCC 14
let's just fix this for the specific case of __class_type_info.
PR c++/113038
gcc/cp/ChangeLog:
* name-lookup.cc (lookup_elaborated_type): Look for bindings
in the global namespace in the ABI namespace.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr106304_b.C: Add dynamic_cast.
---
gcc/cp/name-lookup.cc | 16 +++++++++++++---
gcc/testsuite/g++.dg/modules/pr106304_b.C | 1 +
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 26c6bc71e99..d827d337d3b 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -8089,9 +8089,19 @@ lookup_elaborated_type (tree name, TAG_how how)
{
/* We're in the global module, perhaps there's a tag
there? */
- // FIXME: This isn't quite right, if we find something
- // here, from the language PoV we're not supposed to
- // know it?
+
+ /* FIXME: In general we should probably merge global module
+ classes in check_module_override rather than here, but for
+ GCC14 let's just fix lazy declarations of __class_type_info in
+ build_dynamic_cast_1. */
+ if (current_namespace == abi_node)
+ {
+ tree g = (BINDING_VECTOR_CLUSTER (*slot, 0)
+ .slots[BINDING_SLOT_GLOBAL]);
+ for (ovl_iterator iter (g); iter; ++iter)
+ if (qualify_lookup (*iter, LOOK_want::TYPE))
+ return *iter;
+ }
}
}
}
diff --git a/gcc/testsuite/g++.dg/modules/pr106304_b.C b/gcc/testsuite/g++.dg/modules/pr106304_b.C
index e8333909c8d..0d1da086176 100644
--- a/gcc/testsuite/g++.dg/modules/pr106304_b.C
+++ b/gcc/testsuite/g++.dg/modules/pr106304_b.C
@@ -5,4 +5,5 @@ module pr106304;
void f(A& a) {
as_b(a);
+ dynamic_cast<B*>(&a);
}
--
2.39.3
