On 5/26/24 09:01, Nathaniel Shead wrote:
Is this approach OK? Alternatively I suppose we could do a deep copy of
the overload list when this occurs to ensure we don't affect existing
referents, would that be preferable?
This strategy makes sense, but I have other concerns:
Bootstrapped and regtested (so far just modules.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
-- >8 --
Doing 'remove_node' here is not safe, because it not only mutates the
OVERLOAD we're walking over but potentially any other references to this
OVERLOAD that are cached from phase-1 template lookup. This causes the
attached testcase to fail because the overload set in X::test no longer
contains the 'ns::foo' template once instantiated at the end of the
It looks like ns::foo has been renamed to just f in the testcase.
file.
This patch works around this by simply not removing the old declaration.
This does make the overload list potentially longer than it otherwise
would have been, but only when re-exporting the same set of functions in
a using-decl. Additionally, because 'ovl_insert' always prepends these
newly inserted overloads, repeated exported using-decls won't continue
to add declarations, as the first exported using-decl will be found
before the original (unexported) declaration.
PR c++/114867
gcc/cp/ChangeLog:
* name-lookup.cc (do_nonmember_using_decl): Don't remove the
existing overload.
gcc/testsuite/ChangeLog:
* g++.dg/modules/using-17_a.C: New test.
* g++.dg/modules/using-17_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/name-lookup.cc | 24 +++++++-----------
gcc/testsuite/g++.dg/modules/using-17_a.C | 31 +++++++++++++++++++++++
gcc/testsuite/g++.dg/modules/using-17_b.C | 13 ++++++++++
3 files changed, 53 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/using-17_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/using-17_b.C
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index f1f8c19feb1..130a0e6b5db 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -5231,25 +5231,19 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
if (new_fn == old_fn)
{
- /* The function already exists in the current
- namespace. We will still want to insert it if
- it is revealing a not-revealed thing. */
+ /* The function already exists in the current namespace. */
found = true;
- if (!revealing_p)
- ;
- else if (old.using_p ())
+ if (exporting)
{
- if (exporting)
+ if (old.using_p ())
/* Update in place. 'tis ok. */
OVL_EXPORT_P (old.get_using ()) = true;
- ;
- }
- else if (DECL_MODULE_EXPORT_P (new_fn))
- ;
- else
- {
- value = old.remove_node (value);
- found = false;
+ else if (!DECL_MODULE_EXPORT_P (new_fn))
+ /* We need to re-insert this function as an exported
+ declaration. We can't remove the existing decl
+ because that will change any overloads cached in
+ template functions. */
+ found = false;
What if we're revealing without exporting? That is, a using-declaration
in module purview that isn't exported? Such a declaration should still
prevent discarding, which is my understanding of the use of "revealing"
here.
It seems like the current code already gets that wrong for e.g.
M_1.C:
module;
struct A {};
inline int f() { return 42; }
export module M;
using ::A;
using ::f;
M_2.C:
import M;
inline int f();
struct A a; // { dg-bogus "incomplete" }
int main() {
return f(); // { dg-bogus "undefined" }
}
It looks like part of the problem is that add_binding_entity is only
interested in exported usings, but I think it should also handle
revealing ones.
Jason