| Message ID | aWCgQ4NNsvoAEUVV@Thaum.localdomain |
|---|---|
| State | New |
| Headers |
Return-Path: <gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id BC0034BA2E22 for <patchwork@sourceware.org>; Fri, 9 Jan 2026 06:30:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BC0034BA2E22 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=iYXwEfX8 X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-f54.google.com (mail-pj1-f54.google.com [209.85.216.54]) by sourceware.org (Postfix) with ESMTPS id EAED44BA2E04 for <gcc-patches@gcc.gnu.org>; Fri, 9 Jan 2026 06:29:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EAED44BA2E04 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org EAED44BA2E04 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.216.54 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767940169; cv=none; b=vCaMgB6Q0ZNhm62eT2mkdw06oPRBI7TRnDrEl1psOvecm/QnYBuw6lLuZCkmeC+yPtsvK8nCkQ6mgtfs1P2Tz+dmJVtyrpScOp2tTheZ3dq8BpmCfuW0NdDzttRkA647JkL7RxwdoT3cYfl6hFB6BdBFqPp+0sa6jWLYhU2pFIk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767940169; c=relaxed/simple; bh=SXX/C4EZcJdERiRRGFPd028p/i6L+ishnm50tnoxFGQ=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=eh+/oD66xPcz4XqlSrewgjXtU9y1jkLGc0eFFZMFt9oOLPkbw0AS9ZqqK92av50hL1xQw/6lpUaRLdSflBmYQmKiBN976HngsDtx1iWCQCFMm/h1OYEiUjLqfdruWEOCiB3ocJpU1wqfkhelXwWJY5sOcrRK1GC0VsTl7UoQUgo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EAED44BA2E04 Received: by mail-pj1-f54.google.com with SMTP id 98e67ed59e1d1-34e825b0c42so225974a91.1 for <gcc-patches@gcc.gnu.org>; Thu, 08 Jan 2026 22:29:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1767940168; x=1768544968; darn=gcc.gnu.org; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=RyH2GP7ciOLd8w57Ii1qjszwdIqOg3cX7o70rhkHaBE=; b=iYXwEfX8Ilit+QKKsz3no1FcZB8tRtQUS5wLnOPeqU0XXLTe5OsfURNymEythG5D0o zZwLgChCNkiOFc0Da7UqDNou55lFy3P5Ahu09n5Cju+FUJVBe0+wiKMBYODLRGTFz4Wo 4y244H+zT+uffjiLqPFo82iuXI2eE3S/rW+yYN1p8+eAt4iNccagiifLnbLmNdu7gbl8 FY8bS4BTjzxc3kuQ4Dt+TBDoZGVbXd0gC3hFKLmGs50z2fYgmCOy1+nRLL4/ILLj5mvj QGbyxYXYpSRTNnkkxDWxGP7Xqd5WxP+EVeNJPAAf+g2jTb4I65S5X1pkBMKoivty6HYM Iguw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767940168; x=1768544968; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RyH2GP7ciOLd8w57Ii1qjszwdIqOg3cX7o70rhkHaBE=; b=IpZrWhfVtdtgxF4DDU3akDkNhhppyvOJUBIPfIE9+rm8tibWjz3VV2K3b8OmlF90xI gUl4iZSkM5/i0YwhsCyb2zbLO21F+vnMXS2TurRCLCoZcVA12Y1QPeHIhumeeROiSCwu dMC/Zwmf7JjicOxWjvubqZcAFrOvCcx4KNTS2mFFNrWbo8UZIX7j0UeTi9gsRNSF24HT XThN4zmNvqWGqhiFHFGpMSwKof9kuerCkK4420gYkkROHs9xBcktsqVJ1fL7MdQrs6mA lhq6zmqDWFpXQWltcrEtFnaSlEN5wbvw0nxYBSi7QMfOlxTGT4V3HHlTFYzdrFq5cU5L AiUQ== X-Gm-Message-State: AOJu0YyajXmsRWJVNeiEXvTuzOiV5U38hWQxL22e4mQhfRnXQ7/eENh2 D7+UVW1lf/pSgz6lnnZu+agnMgb9F3d8NhQal9U5hSXQuPfn+9ZjSVEzpmS/sg== X-Gm-Gg: AY/fxX4aIPWCXOhbYAJdDstnL1Oiy/TUhg0TNoQDhcy8Hw8xdGTbZotwVI/U/J0rVv5 vBJLop8erHot7A9wkY9L2c+yhq1xCv7cUSSfvjcdDNz1OkmRw+dOQ9dsQnqgMMdPArBiqPnjg5q adLtpl6epRWZ15/MNQxbkSv9uNxY0gGa8e9YmfpmPq3aQCmB0WuL8j0bdy55mpPoCinWyiRFLH/ eRl0yjvsnEmewwoVpEPN75qgWcaGag7FO6OiL8BMLds/1NfohE69nPnVfdImyyPkA0oZrjmLEnZ 5hH0yEmOGxplevPLDLvKkLF+J/ehWYVyxMU8Gnv6DMyyKmK8iyb8dNAYSs60UQQGJYnZog9dc0o 7d2slgLm3wNy0evA9k79bs/AzO1/mHKKBlpkOZrXgrqoYkMxX0hrFk34Uf/dmutN+LyKBMfIA4t I+PTjaG5q8gq+ZhE28g+QOb4YVvVPtSrE2A2kF7EYClDXH3wXabZgnqXazw/PY4rItA9OIlFLet A== X-Google-Smtp-Source: AGHT+IE10eVDNLKYXQ5vMC4YWek0W5PbTVbEiDSEaXPyLIqBocB1lFVix55n7BfXGO0zqplhiTL+NQ== X-Received: by 2002:a17:90b:4ece:b0:34f:4416:33b5 with SMTP id 98e67ed59e1d1-34f68c49331mr6489122a91.8.1767940167745; Thu, 08 Jan 2026 22:29:27 -0800 (PST) Received: from Thaum.localdomain (163-53-146-3.ip4.superloop.au. [163.53.146.3]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-34f5f88e968sm9672954a91.9.2026.01.08.22.29.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jan 2026 22:29:27 -0800 (PST) Date: Fri, 9 Jan 2026 17:29:23 +1100 From: Nathaniel Shead <nathanieloshead@gmail.com> To: gcc-patches@gcc.gnu.org Cc: Jason Merrill <jason@redhat.com> Subject: [PATCH] c++/modules: Search instantiation context for all namespace name lookup [PR122609] Message-ID: <aWCgQ4NNsvoAEUVV@Thaum.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_BLOCKED, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org |
| Series |
c++/modules: Search instantiation context for all namespace name lookup [PR122609]
|
|
Commit Message
Nathaniel Shead
Jan. 9, 2026, 6:29 a.m. UTC
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? I'm not
100% sure if this matches the wording in the standard, but it more
closely matches Clang's current behaviour and seems to fix a lot of
issues I've seen people have with GCC's current behaviour.
-- >8 --
Many expressions rely on standard library names being visible when used,
such as for structured bindings, typeid, coroutine traits, and so forth.
When these expressions occur in templates, and the instantiations occur
in a TU where those names are not visible, we currently error, even if
the name was visible in the TU the template was defined.
This is a poor user experience (made worse by fixit suggestions to add
an include in the module the template was defined, which wouldn't fix
the error anyway). Instead, I think it seems reasonable (and probably
the intention of the wording?) that we should perform the lookup in the
instantiation context of the lookup.
When using 'import std' this should fix all such errors. If using
traditional #includes to provide the standard library this may or may
not fix the error; in many cases we will still discard the relevant
names (e.g. typeid in a template does not currently cause us to consider
std::type_info to be decl-reachable).
This also fixes the XFAIL in adl-12_b.C as we add_candidates now
properly considers names visible in the instantiation context of the
comparison.
PR c++/122609
gcc/cp/ChangeLog:
* name-lookup.cc (name_lookup::search_namespace_only): Find all
names visible in the instantiation context.
gcc/testsuite/ChangeLog:
* g++.dg/modules/adl-12_b.C: Remove XFAIL.
* g++.dg/modules/inst-8_a.C: New test.
* g++.dg/modules/inst-8_b.C: New test.
* g++.dg/modules/inst-8_c.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/name-lookup.cc | 13 +++++---
gcc/testsuite/g++.dg/modules/adl-12_b.C | 7 ++---
gcc/testsuite/g++.dg/modules/inst-8_a.C | 27 ++++++++++++++++
gcc/testsuite/g++.dg/modules/inst-8_b.C | 42 +++++++++++++++++++++++++
gcc/testsuite/g++.dg/modules/inst-8_c.C | 12 +++++++
5 files changed, 92 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_c.C
Comments
On 1/9/26 2:29 PM, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? I'm not > 100% sure if this matches the wording in the standard, but it more > closely matches Clang's current behaviour and seems to fix a lot of > issues I've seen people have with GCC's current behaviour. > > -- >8 -- > > Many expressions rely on standard library names being visible when used, > such as for structured bindings, typeid, coroutine traits, and so forth. > When these expressions occur in templates, and the instantiations occur > in a TU where those names are not visible, we currently error, even if > the name was visible in the TU the template was defined. > > This is a poor user experience (made worse by fixit suggestions to add > an include in the module the template was defined, which wouldn't fix > the error anyway). Instead, I think it seems reasonable (and probably > the intention of the wording?) that we should perform the lookup in the > instantiation context of the lookup. It seems to me that the lookup should occur in the definition context of the template, both binding the name and making the definition decl-reachable. The full instantiation path is more than we need. > When using 'import std' this should fix all such errors. If using > traditional #includes to provide the standard library this may or may > not fix the error; in many cases we will still discard the relevant > names (e.g. typeid in a template does not currently cause us to consider > std::type_info to be decl-reachable). >> This also fixes the XFAIL in adl-12_b.C as we add_candidates now > properly considers names visible in the instantiation context of the > comparison. Similarly those declarations should be decl-reachable under https://eel.is/c++draft/module.global.frag#note-1 > PR c++/122609 > > gcc/cp/ChangeLog: > > * name-lookup.cc (name_lookup::search_namespace_only): Find all > names visible in the instantiation context. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/adl-12_b.C: Remove XFAIL. > * g++.dg/modules/inst-8_a.C: New test. > * g++.dg/modules/inst-8_b.C: New test. > * g++.dg/modules/inst-8_c.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/name-lookup.cc | 13 +++++--- > gcc/testsuite/g++.dg/modules/adl-12_b.C | 7 ++--- > gcc/testsuite/g++.dg/modules/inst-8_a.C | 27 ++++++++++++++++ > gcc/testsuite/g++.dg/modules/inst-8_b.C | 42 +++++++++++++++++++++++++ > gcc/testsuite/g++.dg/modules/inst-8_c.C | 12 +++++++ > 5 files changed, 92 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_c.C > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index 5d93c13265d..549a04df57c 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -861,7 +861,11 @@ name_lookup::search_namespace_only (tree scope) > /* I presume the binding list is going to be sparser than > the import bitmap. Hence iterate over the former > checking for bits set in the bitmap. */ > - bitmap imports = get_import_bitmap (); > + /* Similarly to with ADL, it seems most helpful to consider > + any declaration visible in the instantiation context as > + being visible for name lookup (PR c++/122609). */ > + bitmap inst_path = NULL; > + bitmap imports = visible_instantiation_path (&inst_path); > binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (val); > int marker = 0; > int dup_detect = 0; > @@ -929,11 +933,11 @@ name_lookup::search_namespace_only (tree scope) > > found:; > /* Is it loaded? */ > + unsigned mod = cluster->indices[jx].base; > if (cluster->slots[jx].is_lazy ()) > { > gcc_assert (cluster->indices[jx].span == 1); > - lazy_load_binding (cluster->indices[jx].base, > - scope, name, &cluster->slots[jx]); > + lazy_load_binding (mod, scope, name, &cluster->slots[jx]); > } > tree bind = cluster->slots[jx]; > if (!bind) > @@ -966,7 +970,8 @@ name_lookup::search_namespace_only (tree scope) > dup_detect |= dup; > } > > - if (bool (want & LOOK_want::ANY_REACHABLE)) > + if (bool (want & LOOK_want::ANY_REACHABLE) > + || (inst_path && bitmap_bit_p (inst_path, mod))) > { > type = STAT_TYPE (bind); > bind = STAT_DECL (bind); > diff --git a/gcc/testsuite/g++.dg/modules/adl-12_b.C b/gcc/testsuite/g++.dg/modules/adl-12_b.C > index cde8c35b20e..abe772d49c7 100644 > --- a/gcc/testsuite/g++.dg/modules/adl-12_b.C > +++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C > @@ -20,10 +20,7 @@ void test(Q::X x) { > > #if __cpp_impl_three_way_comparison >= 201907L > rewrite_ops(0); // OK > - > - // This should error, but lookup_qualified_name in add_candidates > - // doesn't look in the instantiation context of this call, so > - // we don't see the operator!= and think we can validly rewrite. > - rewrite_ops_error(0); // { dg-error "required from here" "PR122609" { xfail *-*-* } } > + rewrite_ops_error(0); // { dg-message "required from here" "" { target c++20 } } > + // { dg-prune-output "no match for" } > #endif > } > diff --git a/gcc/testsuite/g++.dg/modules/inst-8_a.C b/gcc/testsuite/g++.dg/modules/inst-8_a.C > new file mode 100644 > index 00000000000..1b0a47978cb > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/inst-8_a.C > @@ -0,0 +1,27 @@ > +// PR c++/122609 > +// { dg-do compile { target c++20 } } > +// { dg-additional-options "-fmodules" } > +// { dg-module-cmi std } > + > +module; > +#include <coroutine> > +#include <tuple> > +#include <typeinfo> > +export module std; > + > +// PR c++/101140 > +export using ::operator new; > +export using ::operator delete; > + > +export namespace std { > + using std::coroutine_handle; > + using std::coroutine_traits; > + using std::suspend_always; > + > + using std::get; > + using std::tuple; > + using std::tuple_size; > + using std::tuple_element; > + > + using std::type_info; > +} > diff --git a/gcc/testsuite/g++.dg/modules/inst-8_b.C b/gcc/testsuite/g++.dg/modules/inst-8_b.C > new file mode 100644 > index 00000000000..4e3b63da671 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/inst-8_b.C > @@ -0,0 +1,42 @@ > +// PR c++/122609 > +// { dg-do compile { target c++20 } } > +// { dg-additional-options "-fmodules" } > +// { dg-module-cmi M } > + > +export module M; > +import std; // not exported > + > +export template <typename T> void structured_binding() { > + std::tuple<T> t; > + auto [x] = t; > +} > + > +export template <typename T> void operator_new() { > + // PR c++/101140 > + T x; > + new (&x) T; > +} > + > +export template <typename T> void use_typeid() { > + const auto& id = typeid(T); > +} > + > +struct simple_promise; > +struct simple_coroutine : std::coroutine_handle<simple_promise> { > + using promise_type = ::simple_promise; > +}; > +struct simple_promise { > + simple_coroutine get_return_object() { return { simple_coroutine::from_promise(*this) }; } > + std::suspend_always initial_suspend() noexcept { return {}; } > + std::suspend_always final_suspend() noexcept { return {}; } > + void return_void() {} > + void unhandled_exception() {} > +}; > +template <typename T> simple_coroutine coroutine_impl() { > + co_return; > +} > +export template <typename T> void coroutine() { > + simple_coroutine sc = coroutine_impl<T>(); > + sc.resume(); > + sc.destroy(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/inst-8_c.C b/gcc/testsuite/g++.dg/modules/inst-8_c.C > new file mode 100644 > index 00000000000..460232d5a87 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/inst-8_c.C > @@ -0,0 +1,12 @@ > +// PR c++/122609 > +// { dg-do compile { target c++20 } } > +// { dg-additional-options "-fmodules" } > + > +import M; > + > +int main() { > + structured_binding<int>(); > + operator_new<int>(); > + use_typeid<int>(); > + coroutine<int>(); > +}
On Tue, Jan 13, 2026 at 10:45:03PM +0800, Jason Merrill wrote: > On 1/9/26 2:29 PM, Nathaniel Shead wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? I'm not > > 100% sure if this matches the wording in the standard, but it more > > closely matches Clang's current behaviour and seems to fix a lot of > > issues I've seen people have with GCC's current behaviour. > > > > -- >8 -- > > > > Many expressions rely on standard library names being visible when used, > > such as for structured bindings, typeid, coroutine traits, and so forth. > > When these expressions occur in templates, and the instantiations occur > > in a TU where those names are not visible, we currently error, even if > > the name was visible in the TU the template was defined. > > > > This is a poor user experience (made worse by fixit suggestions to add > > an include in the module the template was defined, which wouldn't fix > > the error anyway). Instead, I think it seems reasonable (and probably > > the intention of the wording?) that we should perform the lookup in the > > instantiation context of the lookup. > > It seems to me that the lookup should occur in the definition context of the > template, both binding the name and making the definition decl-reachable. > The full instantiation path is more than we need. > I've had a bit of a look into this; would this mean perhaps extending 'DEPENDENT_OPERATOR_TYPE' (or something like it) to save the results of name lookup at definition time for all contexts that we currently do 'lookup_qualified_name' somewhere if processing_template_decl, and then at instantiation time supplementing the results with a second lookup and using that set? Or is there another approach that you're thinking of here? > > When using 'import std' this should fix all such errors. If using > > traditional #includes to provide the standard library this may or may > > not fix the error; in many cases we will still discard the relevant > > names (e.g. typeid in a template does not currently cause us to consider > > std::type_info to be decl-reachable). > > > This also fixes the XFAIL in adl-12_b.C as we add_candidates now > > properly considers names visible in the instantiation context of the > > comparison. > > Similarly those declarations should be decl-reachable under > https://eel.is/c++draft/module.global.frag#note-1 > Yes, we already consider the names decl-reachable (and they are part of the module interface) due to the ADL, they're just not visible from the TU we instantiate from, without either this patch or some other way to perform the lookup in the context of the definition. > > PR c++/122609 > > > > gcc/cp/ChangeLog: > > > > * name-lookup.cc (name_lookup::search_namespace_only): Find all > > names visible in the instantiation context. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/adl-12_b.C: Remove XFAIL. > > * g++.dg/modules/inst-8_a.C: New test. > > * g++.dg/modules/inst-8_b.C: New test. > > * g++.dg/modules/inst-8_c.C: New test. > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > --- > > gcc/cp/name-lookup.cc | 13 +++++--- > > gcc/testsuite/g++.dg/modules/adl-12_b.C | 7 ++--- > > gcc/testsuite/g++.dg/modules/inst-8_a.C | 27 ++++++++++++++++ > > gcc/testsuite/g++.dg/modules/inst-8_b.C | 42 +++++++++++++++++++++++++ > > gcc/testsuite/g++.dg/modules/inst-8_c.C | 12 +++++++ > > 5 files changed, 92 insertions(+), 9 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_c.C > > > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > > index 5d93c13265d..549a04df57c 100644 > > --- a/gcc/cp/name-lookup.cc > > +++ b/gcc/cp/name-lookup.cc > > @@ -861,7 +861,11 @@ name_lookup::search_namespace_only (tree scope) > > /* I presume the binding list is going to be sparser than > > the import bitmap. Hence iterate over the former > > checking for bits set in the bitmap. */ > > - bitmap imports = get_import_bitmap (); > > + /* Similarly to with ADL, it seems most helpful to consider > > + any declaration visible in the instantiation context as > > + being visible for name lookup (PR c++/122609). */ > > + bitmap inst_path = NULL; > > + bitmap imports = visible_instantiation_path (&inst_path); > > binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (val); > > int marker = 0; > > int dup_detect = 0; > > @@ -929,11 +933,11 @@ name_lookup::search_namespace_only (tree scope) > > found:; > > /* Is it loaded? */ > > + unsigned mod = cluster->indices[jx].base; > > if (cluster->slots[jx].is_lazy ()) > > { > > gcc_assert (cluster->indices[jx].span == 1); > > - lazy_load_binding (cluster->indices[jx].base, > > - scope, name, &cluster->slots[jx]); > > + lazy_load_binding (mod, scope, name, &cluster->slots[jx]); > > } > > tree bind = cluster->slots[jx]; > > if (!bind) > > @@ -966,7 +970,8 @@ name_lookup::search_namespace_only (tree scope) > > dup_detect |= dup; > > } > > - if (bool (want & LOOK_want::ANY_REACHABLE)) > > + if (bool (want & LOOK_want::ANY_REACHABLE) > > + || (inst_path && bitmap_bit_p (inst_path, mod))) > > { > > type = STAT_TYPE (bind); > > bind = STAT_DECL (bind); > > diff --git a/gcc/testsuite/g++.dg/modules/adl-12_b.C b/gcc/testsuite/g++.dg/modules/adl-12_b.C > > index cde8c35b20e..abe772d49c7 100644 > > --- a/gcc/testsuite/g++.dg/modules/adl-12_b.C > > +++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C > > @@ -20,10 +20,7 @@ void test(Q::X x) { > > #if __cpp_impl_three_way_comparison >= 201907L > > rewrite_ops(0); // OK > > - > > - // This should error, but lookup_qualified_name in add_candidates > > - // doesn't look in the instantiation context of this call, so > > - // we don't see the operator!= and think we can validly rewrite. > > - rewrite_ops_error(0); // { dg-error "required from here" "PR122609" { xfail *-*-* } } > > + rewrite_ops_error(0); // { dg-message "required from here" "" { target c++20 } } > > + // { dg-prune-output "no match for" } > > #endif > > } > > diff --git a/gcc/testsuite/g++.dg/modules/inst-8_a.C b/gcc/testsuite/g++.dg/modules/inst-8_a.C > > new file mode 100644 > > index 00000000000..1b0a47978cb > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/inst-8_a.C > > @@ -0,0 +1,27 @@ > > +// PR c++/122609 > > +// { dg-do compile { target c++20 } } > > +// { dg-additional-options "-fmodules" } > > +// { dg-module-cmi std } > > + > > +module; > > +#include <coroutine> > > +#include <tuple> > > +#include <typeinfo> > > +export module std; > > + > > +// PR c++/101140 > > +export using ::operator new; > > +export using ::operator delete; > > + > > +export namespace std { > > + using std::coroutine_handle; > > + using std::coroutine_traits; > > + using std::suspend_always; > > + > > + using std::get; > > + using std::tuple; > > + using std::tuple_size; > > + using std::tuple_element; > > + > > + using std::type_info; > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/inst-8_b.C b/gcc/testsuite/g++.dg/modules/inst-8_b.C > > new file mode 100644 > > index 00000000000..4e3b63da671 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/inst-8_b.C > > @@ -0,0 +1,42 @@ > > +// PR c++/122609 > > +// { dg-do compile { target c++20 } } > > +// { dg-additional-options "-fmodules" } > > +// { dg-module-cmi M } > > + > > +export module M; > > +import std; // not exported > > + > > +export template <typename T> void structured_binding() { > > + std::tuple<T> t; > > + auto [x] = t; > > +} > > + > > +export template <typename T> void operator_new() { > > + // PR c++/101140 > > + T x; > > + new (&x) T; > > +} > > + > > +export template <typename T> void use_typeid() { > > + const auto& id = typeid(T); > > +} > > + > > +struct simple_promise; > > +struct simple_coroutine : std::coroutine_handle<simple_promise> { > > + using promise_type = ::simple_promise; > > +}; > > +struct simple_promise { > > + simple_coroutine get_return_object() { return { simple_coroutine::from_promise(*this) }; } > > + std::suspend_always initial_suspend() noexcept { return {}; } > > + std::suspend_always final_suspend() noexcept { return {}; } > > + void return_void() {} > > + void unhandled_exception() {} > > +}; > > +template <typename T> simple_coroutine coroutine_impl() { > > + co_return; > > +} > > +export template <typename T> void coroutine() { > > + simple_coroutine sc = coroutine_impl<T>(); > > + sc.resume(); > > + sc.destroy(); > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/inst-8_c.C b/gcc/testsuite/g++.dg/modules/inst-8_c.C > > new file mode 100644 > > index 00000000000..460232d5a87 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/inst-8_c.C > > @@ -0,0 +1,12 @@ > > +// PR c++/122609 > > +// { dg-do compile { target c++20 } } > > +// { dg-additional-options "-fmodules" } > > + > > +import M; > > + > > +int main() { > > + structured_binding<int>(); > > + operator_new<int>(); > > + use_typeid<int>(); > > + coroutine<int>(); > > +} >
On 1/24/26 12:26 PM, Nathaniel Shead wrote: > On Tue, Jan 13, 2026 at 10:45:03PM +0800, Jason Merrill wrote: >> On 1/9/26 2:29 PM, Nathaniel Shead wrote: >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? I'm not >>> 100% sure if this matches the wording in the standard, but it more >>> closely matches Clang's current behaviour and seems to fix a lot of >>> issues I've seen people have with GCC's current behaviour. >>> >>> -- >8 -- >>> >>> Many expressions rely on standard library names being visible when used, >>> such as for structured bindings, typeid, coroutine traits, and so forth. >>> When these expressions occur in templates, and the instantiations occur >>> in a TU where those names are not visible, we currently error, even if >>> the name was visible in the TU the template was defined. >>> >>> This is a poor user experience (made worse by fixit suggestions to add >>> an include in the module the template was defined, which wouldn't fix >>> the error anyway). Instead, I think it seems reasonable (and probably >>> the intention of the wording?) that we should perform the lookup in the >>> instantiation context of the lookup. >> >> It seems to me that the lookup should occur in the definition context of the >> template, both binding the name and making the definition decl-reachable. >> The full instantiation path is more than we need. > > I've had a bit of a look into this; would this mean perhaps extending > 'DEPENDENT_OPERATOR_TYPE' (or something like it) to save the results of > name lookup at definition time for all contexts that we currently do > 'lookup_qualified_name' somewhere if processing_template_decl, and then > at instantiation time supplementing the results with a second lookup and > using that set? Or is there another approach that you're thinking of > here? Yes, long-term that seems like how it ought to work to handle order of declaration edge cases. Easier should be to do something like this patch, but only looking at the originating module of the current instantiation rather than the full instantiation path. >>> When using 'import std' this should fix all such errors. If using >>> traditional #includes to provide the standard library this may or may >>> not fix the error; in many cases we will still discard the relevant >>> names (e.g. typeid in a template does not currently cause us to consider >>> std::type_info to be decl-reachable). >>>> This also fixes the XFAIL in adl-12_b.C as we add_candidates now >>> properly considers names visible in the instantiation context of the >>> comparison. >> >> Similarly those declarations should be decl-reachable under >> https://eel.is/c++draft/module.global.frag#note-1 >> > > Yes, we already consider the names decl-reachable (and they are part of > the module interface) due to the ADL, they're just not visible from the > TU we instantiate from, without either this patch or some other way to > perform the lookup in the context of the definition. > >>> PR c++/122609 >>> >>> gcc/cp/ChangeLog: >>> >>> * name-lookup.cc (name_lookup::search_namespace_only): Find all >>> names visible in the instantiation context. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/modules/adl-12_b.C: Remove XFAIL. >>> * g++.dg/modules/inst-8_a.C: New test. >>> * g++.dg/modules/inst-8_b.C: New test. >>> * g++.dg/modules/inst-8_c.C: New test. >>> >>> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> >>> --- >>> gcc/cp/name-lookup.cc | 13 +++++--- >>> gcc/testsuite/g++.dg/modules/adl-12_b.C | 7 ++--- >>> gcc/testsuite/g++.dg/modules/inst-8_a.C | 27 ++++++++++++++++ >>> gcc/testsuite/g++.dg/modules/inst-8_b.C | 42 +++++++++++++++++++++++++ >>> gcc/testsuite/g++.dg/modules/inst-8_c.C | 12 +++++++ >>> 5 files changed, 92 insertions(+), 9 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_a.C >>> create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_b.C >>> create mode 100644 gcc/testsuite/g++.dg/modules/inst-8_c.C >>> >>> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc >>> index 5d93c13265d..549a04df57c 100644 >>> --- a/gcc/cp/name-lookup.cc >>> +++ b/gcc/cp/name-lookup.cc >>> @@ -861,7 +861,11 @@ name_lookup::search_namespace_only (tree scope) >>> /* I presume the binding list is going to be sparser than >>> the import bitmap. Hence iterate over the former >>> checking for bits set in the bitmap. */ >>> - bitmap imports = get_import_bitmap (); >>> + /* Similarly to with ADL, it seems most helpful to consider >>> + any declaration visible in the instantiation context as >>> + being visible for name lookup (PR c++/122609). */ >>> + bitmap inst_path = NULL; >>> + bitmap imports = visible_instantiation_path (&inst_path); >>> binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (val); >>> int marker = 0; >>> int dup_detect = 0; >>> @@ -929,11 +933,11 @@ name_lookup::search_namespace_only (tree scope) >>> found:; >>> /* Is it loaded? */ >>> + unsigned mod = cluster->indices[jx].base; >>> if (cluster->slots[jx].is_lazy ()) >>> { >>> gcc_assert (cluster->indices[jx].span == 1); >>> - lazy_load_binding (cluster->indices[jx].base, >>> - scope, name, &cluster->slots[jx]); >>> + lazy_load_binding (mod, scope, name, &cluster->slots[jx]); >>> } >>> tree bind = cluster->slots[jx]; >>> if (!bind) >>> @@ -966,7 +970,8 @@ name_lookup::search_namespace_only (tree scope) >>> dup_detect |= dup; >>> } >>> - if (bool (want & LOOK_want::ANY_REACHABLE)) >>> + if (bool (want & LOOK_want::ANY_REACHABLE) >>> + || (inst_path && bitmap_bit_p (inst_path, mod))) >>> { >>> type = STAT_TYPE (bind); >>> bind = STAT_DECL (bind); >>> diff --git a/gcc/testsuite/g++.dg/modules/adl-12_b.C b/gcc/testsuite/g++.dg/modules/adl-12_b.C >>> index cde8c35b20e..abe772d49c7 100644 >>> --- a/gcc/testsuite/g++.dg/modules/adl-12_b.C >>> +++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C >>> @@ -20,10 +20,7 @@ void test(Q::X x) { >>> #if __cpp_impl_three_way_comparison >= 201907L >>> rewrite_ops(0); // OK >>> - >>> - // This should error, but lookup_qualified_name in add_candidates >>> - // doesn't look in the instantiation context of this call, so >>> - // we don't see the operator!= and think we can validly rewrite. >>> - rewrite_ops_error(0); // { dg-error "required from here" "PR122609" { xfail *-*-* } } >>> + rewrite_ops_error(0); // { dg-message "required from here" "" { target c++20 } } >>> + // { dg-prune-output "no match for" } >>> #endif >>> } >>> diff --git a/gcc/testsuite/g++.dg/modules/inst-8_a.C b/gcc/testsuite/g++.dg/modules/inst-8_a.C >>> new file mode 100644 >>> index 00000000000..1b0a47978cb >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/modules/inst-8_a.C >>> @@ -0,0 +1,27 @@ >>> +// PR c++/122609 >>> +// { dg-do compile { target c++20 } } >>> +// { dg-additional-options "-fmodules" } >>> +// { dg-module-cmi std } >>> + >>> +module; >>> +#include <coroutine> >>> +#include <tuple> >>> +#include <typeinfo> >>> +export module std; >>> + >>> +// PR c++/101140 >>> +export using ::operator new; >>> +export using ::operator delete; >>> + >>> +export namespace std { >>> + using std::coroutine_handle; >>> + using std::coroutine_traits; >>> + using std::suspend_always; >>> + >>> + using std::get; >>> + using std::tuple; >>> + using std::tuple_size; >>> + using std::tuple_element; >>> + >>> + using std::type_info; >>> +} >>> diff --git a/gcc/testsuite/g++.dg/modules/inst-8_b.C b/gcc/testsuite/g++.dg/modules/inst-8_b.C >>> new file mode 100644 >>> index 00000000000..4e3b63da671 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/modules/inst-8_b.C >>> @@ -0,0 +1,42 @@ >>> +// PR c++/122609 >>> +// { dg-do compile { target c++20 } } >>> +// { dg-additional-options "-fmodules" } >>> +// { dg-module-cmi M } >>> + >>> +export module M; >>> +import std; // not exported >>> + >>> +export template <typename T> void structured_binding() { >>> + std::tuple<T> t; >>> + auto [x] = t; >>> +} >>> + >>> +export template <typename T> void operator_new() { >>> + // PR c++/101140 >>> + T x; >>> + new (&x) T; >>> +} >>> + >>> +export template <typename T> void use_typeid() { >>> + const auto& id = typeid(T); >>> +} >>> + >>> +struct simple_promise; >>> +struct simple_coroutine : std::coroutine_handle<simple_promise> { >>> + using promise_type = ::simple_promise; >>> +}; >>> +struct simple_promise { >>> + simple_coroutine get_return_object() { return { simple_coroutine::from_promise(*this) }; } >>> + std::suspend_always initial_suspend() noexcept { return {}; } >>> + std::suspend_always final_suspend() noexcept { return {}; } >>> + void return_void() {} >>> + void unhandled_exception() {} >>> +}; >>> +template <typename T> simple_coroutine coroutine_impl() { >>> + co_return; >>> +} >>> +export template <typename T> void coroutine() { >>> + simple_coroutine sc = coroutine_impl<T>(); >>> + sc.resume(); >>> + sc.destroy(); >>> +} >>> diff --git a/gcc/testsuite/g++.dg/modules/inst-8_c.C b/gcc/testsuite/g++.dg/modules/inst-8_c.C >>> new file mode 100644 >>> index 00000000000..460232d5a87 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/modules/inst-8_c.C >>> @@ -0,0 +1,12 @@ >>> +// PR c++/122609 >>> +// { dg-do compile { target c++20 } } >>> +// { dg-additional-options "-fmodules" } >>> + >>> +import M; >>> + >>> +int main() { >>> + structured_binding<int>(); >>> + operator_new<int>(); >>> + use_typeid<int>(); >>> + coroutine<int>(); >>> +} >> >
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index 5d93c13265d..549a04df57c 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -861,7 +861,11 @@ name_lookup::search_namespace_only (tree scope) /* I presume the binding list is going to be sparser than the import bitmap. Hence iterate over the former checking for bits set in the bitmap. */ - bitmap imports = get_import_bitmap (); + /* Similarly to with ADL, it seems most helpful to consider + any declaration visible in the instantiation context as + being visible for name lookup (PR c++/122609). */ + bitmap inst_path = NULL; + bitmap imports = visible_instantiation_path (&inst_path); binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (val); int marker = 0; int dup_detect = 0; @@ -929,11 +933,11 @@ name_lookup::search_namespace_only (tree scope) found:; /* Is it loaded? */ + unsigned mod = cluster->indices[jx].base; if (cluster->slots[jx].is_lazy ()) { gcc_assert (cluster->indices[jx].span == 1); - lazy_load_binding (cluster->indices[jx].base, - scope, name, &cluster->slots[jx]); + lazy_load_binding (mod, scope, name, &cluster->slots[jx]); } tree bind = cluster->slots[jx]; if (!bind) @@ -966,7 +970,8 @@ name_lookup::search_namespace_only (tree scope) dup_detect |= dup; } - if (bool (want & LOOK_want::ANY_REACHABLE)) + if (bool (want & LOOK_want::ANY_REACHABLE) + || (inst_path && bitmap_bit_p (inst_path, mod))) { type = STAT_TYPE (bind); bind = STAT_DECL (bind); diff --git a/gcc/testsuite/g++.dg/modules/adl-12_b.C b/gcc/testsuite/g++.dg/modules/adl-12_b.C index cde8c35b20e..abe772d49c7 100644 --- a/gcc/testsuite/g++.dg/modules/adl-12_b.C +++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C @@ -20,10 +20,7 @@ void test(Q::X x) { #if __cpp_impl_three_way_comparison >= 201907L rewrite_ops(0); // OK - - // This should error, but lookup_qualified_name in add_candidates - // doesn't look in the instantiation context of this call, so - // we don't see the operator!= and think we can validly rewrite. - rewrite_ops_error(0); // { dg-error "required from here" "PR122609" { xfail *-*-* } } + rewrite_ops_error(0); // { dg-message "required from here" "" { target c++20 } } + // { dg-prune-output "no match for" } #endif } diff --git a/gcc/testsuite/g++.dg/modules/inst-8_a.C b/gcc/testsuite/g++.dg/modules/inst-8_a.C new file mode 100644 index 00000000000..1b0a47978cb --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/inst-8_a.C @@ -0,0 +1,27 @@ +// PR c++/122609 +// { dg-do compile { target c++20 } } +// { dg-additional-options "-fmodules" } +// { dg-module-cmi std } + +module; +#include <coroutine> +#include <tuple> +#include <typeinfo> +export module std; + +// PR c++/101140 +export using ::operator new; +export using ::operator delete; + +export namespace std { + using std::coroutine_handle; + using std::coroutine_traits; + using std::suspend_always; + + using std::get; + using std::tuple; + using std::tuple_size; + using std::tuple_element; + + using std::type_info; +} diff --git a/gcc/testsuite/g++.dg/modules/inst-8_b.C b/gcc/testsuite/g++.dg/modules/inst-8_b.C new file mode 100644 index 00000000000..4e3b63da671 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/inst-8_b.C @@ -0,0 +1,42 @@ +// PR c++/122609 +// { dg-do compile { target c++20 } } +// { dg-additional-options "-fmodules" } +// { dg-module-cmi M } + +export module M; +import std; // not exported + +export template <typename T> void structured_binding() { + std::tuple<T> t; + auto [x] = t; +} + +export template <typename T> void operator_new() { + // PR c++/101140 + T x; + new (&x) T; +} + +export template <typename T> void use_typeid() { + const auto& id = typeid(T); +} + +struct simple_promise; +struct simple_coroutine : std::coroutine_handle<simple_promise> { + using promise_type = ::simple_promise; +}; +struct simple_promise { + simple_coroutine get_return_object() { return { simple_coroutine::from_promise(*this) }; } + std::suspend_always initial_suspend() noexcept { return {}; } + std::suspend_always final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} +}; +template <typename T> simple_coroutine coroutine_impl() { + co_return; +} +export template <typename T> void coroutine() { + simple_coroutine sc = coroutine_impl<T>(); + sc.resume(); + sc.destroy(); +} diff --git a/gcc/testsuite/g++.dg/modules/inst-8_c.C b/gcc/testsuite/g++.dg/modules/inst-8_c.C new file mode 100644 index 00000000000..460232d5a87 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/inst-8_c.C @@ -0,0 +1,12 @@ +// PR c++/122609 +// { dg-do compile { target c++20 } } +// { dg-additional-options "-fmodules" } + +import M; + +int main() { + structured_binding<int>(); + operator_new<int>(); + use_typeid<int>(); + coroutine<int>(); +}