From patchwork Sun May 26 13:01:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1939356 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=afnGNgfU; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4VnJp63jR1z20PT for ; Sun, 26 May 2024 23:02:04 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BC4073858429 for ; Sun, 26 May 2024 13:02:02 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by sourceware.org (Postfix) with ESMTPS id D766C3858D28 for ; Sun, 26 May 2024 13:01:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D766C3858D28 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 D766C3858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::636 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716728506; cv=none; b=OoPNQq14BonBjjVLLwUIAJFJ2HBGg6plwrL/QRQiRiQBO5t3NVBzt2VeReCWM8ajP8xzK7DBaV/DN86CjBjKjq8oqW/jDQt7FRVfZpDv4a6cdEwvYlLQJY0nhd+gwwB8WritZBH4AVDMxdj3x6knMxVPsNS2mNPXa+UAn4yv5JI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716728506; c=relaxed/simple; bh=PegZ4lySkHwoqfi9W31Tc0vBJbpLqkIKmFXY33hF0qg=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=nYbQv49v2VhBfAkz7Kbk2x4qyETvlaF4jT+l4jXQkCXxoQ/Ks5JZh9DQ4gTuHXZIZRUO1CzIBaQbm4/kkYyb6lOZ4gv9Dt4KQ0IgZbsVL/IfUXOEp2Xeix+eES/ZskV2MDNmsLJP53Yq+A3AjZYJk3bCDUl08qnCbeoTEe3p5ck= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x636.google.com with SMTP id d9443c01a7336-1f47f07acd3so5180965ad.0 for ; Sun, 26 May 2024 06:01:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716728504; x=1717333304; darn=gcc.gnu.org; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=vGMma67pDVe+fcy1Um12G2Z6Udo/Mig+94EN/KUgkFM=; b=afnGNgfULeD8KTTF4WqSMYS1YNIy4ML2JJYIVVRnGLWpSZ1ZT9kUlGVmr1/qh/2pBm Am7UcyUv/NJDjGG44ochfZ80zi4Isa6UnBibsR0xQK70aUKoLQcdmJurZXGYh6oBFevV DzZXbBs4prsC7bM0pByc85/Wstf8XlvWjUy/VU8vmmUGjRO7ju7NnMZrsf9mHXgwYuHs 3pxQWVH5l/miR2404W9lTIjFFAdfttxYOK+6psLDsqQyYjB6nH2k3Hu2ElHXnkHSMVuC t6vOnnZCMIvBPc+yysC9QVjHYJcu6Q+kpGAfSI+xScgMYDyNKAU9RGpgrpU2R5Ysa/U4 jw5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716728504; x=1717333304; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vGMma67pDVe+fcy1Um12G2Z6Udo/Mig+94EN/KUgkFM=; b=lNRJsdUg6/DoKFz69veIEC+Gs+GXylqOR6hvCfQOoCDHQySGs4OPXCpIE1FpfD3baa WWUbCE1kvIPgmuEI39URq8RhZzcl1uwtP9G0kRJtk1hA9Z3nWNHsscOscNcJyriOywyH /Zxe3/SZOIFx131FtRA2h8U8dxofK6ZFlOc4MSzDlNDI9WemFyHkQ0BQV1e+PM6rYSJm u0ZRYpRjYyc2Jq3ZbaKi3hq0Jy8lKKdcY1dFx+pxkuIYZtI3Sn+2IEHOKb3AM4q9OXD7 FN/st4X/tYYXS2H4yNwCAt+71GoLKw867D9t7L9KyYKymRqDda5C1lBSya4/jFTHrYbq XC7A== X-Gm-Message-State: AOJu0YyMcBuk02H0e5Te1odCqNK2F4ZTGbT8JccpK/L5yK1yJMxNdsr3 4uVny/8H3uy4L2kPFSaVO9H5twsLn77VMgIWzgSUCPLhEEaBSECVGupCQw== X-Google-Smtp-Source: AGHT+IFD4DTxQyHwYaK+NfnaVrdtzv4Px39kOKw4+mzYPOw8K0H67gngTC6aO7j4gGju41SLQKvPuw== X-Received: by 2002:a17:902:f544:b0:1f4:6215:1b0a with SMTP id d9443c01a7336-1f4621530a5mr59481535ad.50.1716728503443; Sun, 26 May 2024 06:01:43 -0700 (PDT) Received: from Thaum. (14-200-72-253.tpgi.com.au. [14.200.72.253]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f47737d71csm18609605ad.303.2024.05.26.06.01.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 26 May 2024 06:01:42 -0700 (PDT) Message-ID: <665332b6.170a0220.0d48.3d88@mx.google.com> X-Google-Original-Message-ID: Date: Sun, 26 May 2024 23:01:32 +1000 From: Nathaniel Shead To: gcc-patches@gcc.gnu.org Cc: Jason Merrill , Nathan Sidwell Subject: [PATCH] c++/modules: Prevent revealing a using-decl affecting cached overloads [PR114867] MIME-Version: 1.0 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_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org 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? 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 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 --- 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; } break; } diff --git a/gcc/testsuite/g++.dg/modules/using-17_a.C b/gcc/testsuite/g++.dg/modules/using-17_a.C new file mode 100644 index 00000000000..de601ea2be0 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/using-17_a.C @@ -0,0 +1,31 @@ +// PR c++/114867 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi M } + +module; + +namespace ns { + template void f(T); + + namespace inner { + class E {}; + int f(E); + } + using inner::f; +} + +export module M; + +template +struct X { + void test() { ns::f(T{}); } +}; +template struct X; + +export namespace ns { + using ns::f; +} + +export auto get_e() { + return ns::inner::E{}; +} diff --git a/gcc/testsuite/g++.dg/modules/using-17_b.C b/gcc/testsuite/g++.dg/modules/using-17_b.C new file mode 100644 index 00000000000..e31582110e5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/using-17_b.C @@ -0,0 +1,13 @@ +// PR c++/114867 +// { dg-additional-options "-fmodules-ts" } + +import M; + +int main() { + int a = 0; + ns::f(a); + + // Should also still find the inner::f overload + auto e = get_e(); + int r = ns::f(e); +}