From patchwork Mon Sep 4 17:18:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Li, Pan2 via Gcc-patches" X-Patchwork-Id: 137462 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ab0a:0:b0:3f2:4152:657d with SMTP id m10csp1219575vqo; Mon, 4 Sep 2023 10:24:50 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEzfzyrQhldS5RmLHu4zhJlxqesBlvKCnXVIAW8DgEf5TFn+KEcDXsVxH/tDg0MUJhtDD+0 X-Received: by 2002:a17:907:2ce7:b0:9a1:bb8f:17d7 with SMTP id hz7-20020a1709072ce700b009a1bb8f17d7mr6946924ejc.12.1693848290302; Mon, 04 Sep 2023 10:24:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1693848290; cv=none; d=google.com; s=arc-20160816; b=0I/GaJu1AWzN7JZMnVN5E7GQKuh193j2ZfKsppHJGLzfycObeGBOd+Rx5yQTU6O0rY t0rE0bHmNBdCSAcPefaIvBp1LL9Z/sEnuCxLuNlzuEsrEZ/bZF/BhZ49C7hCs22jUwSr mSXgCbuRRUtNWBsfvn4pSp5aLluqac5f7wAkADg518F1QJxlFpXeZlxolM87hm1UYAga xewewZrbCC4AAVbz543cYRchl8RfJ8ghJZ17FBO+yyyOmEO6avSlCmwn9OioWxJ73hYk CVkLG4P7rZ+nut2CueyyWQbolwgYSH/LHWwC/5W+ntwBMKKa0KAP34N+LBwyrlwj0MaP O5Cw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=Rkwb2jB+AX5+e/a1y5bm6sKX/tdszMA5UF4WHgLSX6A=; fh=ZlB0koxfnHOTo42bNZcmZz9kKPmSXops0NJDj86lBGk=; b=gtpaq0KsapebohcM40XiDo2WKyFBMoft5RuO8HCFod1cFy4vXAOA+iR2bDpTeAVCbj bE7XBRtn75+DB2LEtBL5xFg3q3yGkONcjk0WQhO28SJMmKPm7bhJ7Q/JBxW4PPNBzNa0 n2XScUACHJYTduMKNLvknIpjmKstn7og9u1UYJTLJCNGASHc8hQAsnIn1rNq7y7bdWWb TNJRRQ/P3mjNOMDRSM0+0+6SxgO8u6MO46AN1vH+684Bkh/+vfydbiJ5NCvUr4+pXEwr Vz2fkoQr1YvJlhWgbH9aLWqF9GDv3MKTBTh0AKdZ+3xl1ueJATj1BhKym3VRwl3RIFwv kDKg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=GpcpOBkC; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id j5-20020a170906830500b0099c05358e75si6282465ejx.759.2023.09.04.10.24.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 10:24:50 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=GpcpOBkC; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BF64B385800C for ; Mon, 4 Sep 2023 17:24:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BF64B385800C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1693848288; bh=Rkwb2jB+AX5+e/a1y5bm6sKX/tdszMA5UF4WHgLSX6A=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=GpcpOBkCcURUwGnlSMARuSP/DhjUr1T0TDXT44a4AeXtdVoqGe0Gl5aRnOBOiUP9y hcn4dHVBQqSMXiqIbu/hd68g5qtWiQKt1U8Vt16c/5AgneWZjp8Sh8dfLQZzqNNU/Q 7smpATVc7Yrp+VDtXSfgDHEudm8VgF15WGTrbx20= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by sourceware.org (Postfix) with ESMTPS id 9433A3858C30; Mon, 4 Sep 2023 17:23:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9433A3858C30 Received: by mail-wm1-x32a.google.com with SMTP id 5b1f17b1804b1-401d10e3e54so16999735e9.2; Mon, 04 Sep 2023 10:23:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693848238; x=1694453038; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Rkwb2jB+AX5+e/a1y5bm6sKX/tdszMA5UF4WHgLSX6A=; b=f1hHVouwMxQrUIxjD0uEi5p026sBfG1PDi/Cf/4Z/bFxQdt9vW8xHrGxDzZIcQTude iBvxBBaYy58n9v454v3ZKb45InRaGjbqsR525veLVW5zjgLDv5IROY2lJqJgj+b4AR14 XaOh01UlnnsYbYt6pf218tNBXw3fNwnlbHrjaAyb2Wixmp1YAPWuSTSNnlryahka1XA4 M8h9m22uMAZzDcYwDH0ReILynlOfbEkJhLQMJcnFFw8KoAKztUYonXnrVvYI24voptm6 A2XigxcLwLQp/g8J3EvgniWn9wyLQm2oBGkrTxMtrzawcV+1co+dSFmSsAnzJSG1ae16 Jn9A== X-Gm-Message-State: AOJu0Ywzr7Cja2mmcgA3zhqqmOI0IKRVgbyHD+OdhSJRnXOye7f/UG8N xdj0w3kchxD/aGt5sQCYFLQgSYoKM0Jd X-Received: by 2002:a5d:67c7:0:b0:31a:e5e8:1d8e with SMTP id n7-20020a5d67c7000000b0031ae5e81d8emr7902728wrw.5.1693848237983; Mon, 04 Sep 2023 10:23:57 -0700 (PDT) Received: from localhost ([2a01:e0a:2ec:f0d0:e30e:67bf:23ff:df20]) by smtp.gmail.com with UTF8SMTPSA id o4-20020a5d6844000000b003197b85bad2sm15125010wrw.79.2023.09.04.10.23.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Sep 2023 10:23:57 -0700 (PDT) X-Google-Original-From: vultkayn@gcc.gnu.org To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: Additional warning for name-hiding [PR12341] Date: Mon, 4 Sep 2023 19:18:59 +0200 Message-Id: <20230904171858.2660517-1-vultkayn@gcc.gnu.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-10.4 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 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: , X-Patchwork-Original-From: Benjamin Priour via Gcc-patches From: "Li, Pan2 via Gcc-patches" Reply-To: priour.be@gmail.com Cc: benjamin priour Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1776128664710208767 X-GMAIL-MSGID: 1776128664710208767 From: benjamin priour Hi, This patch was the first I wrote and had been at that time returned to me because ill-formatted. Getting busy with other things, I forgot about it. I've now fixed the formatting. Succesfully regstrapped on x86_64-linux-gnu off trunk a7d052b3200c7928d903a0242b8cfd75d131e374. Is it OK for trunk ? Thanks, Benjamin. Patch below. --- Add a new warning for name-hiding. When a class's field is named similarly to one inherited, a warning should be issued. This new warning is controlled by the existing Wshadow. gcc/cp/ChangeLog: PR c++/12341 * search.cc (lookup_member): New optional parameter to preempt processing the inheritance tree deeper than necessary. (lookup_field): Likewise. (dfs_walk_all): Likewise. * cp-tree.h: Update the above declarations. * class.cc: (warn_name_hiding): New function. (finish_struct_1): Call warn_name_hiding if -Wshadow. gcc/testsuite/ChangeLog: PR c++/12341 * g++.dg/pr12341-1.C: New file. * g++.dg/pr12341-2.C: New file. Signed-off-by: benjamin priour --- gcc/cp/class.cc | 75 ++++++++++++++++++++++++++++++++ gcc/cp/cp-tree.h | 9 ++-- gcc/cp/search.cc | 28 ++++++++---- gcc/testsuite/g++.dg/pr12341-1.C | 65 +++++++++++++++++++++++++++ gcc/testsuite/g++.dg/pr12341-2.C | 34 +++++++++++++++ 5 files changed, 200 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr12341-1.C create mode 100644 gcc/testsuite/g++.dg/pr12341-2.C diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index 778759237dc..b1c59c392a0 100644 --- a/gcc/cp/class.cc +++ b/gcc/cp/class.cc @@ -3080,6 +3080,79 @@ warn_hidden (tree t) } } +/* Warn about non-static fields name hiding. */ + +static void +warn_name_hiding (tree t) +{ + if (is_empty_class (t) || CLASSTYPE_NEARLY_EMPTY_P (t)) + return; + + for (tree field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field)) + { + /* Skip if field is not an user-defined non-static data member. */ + if (TREE_CODE (field) != FIELD_DECL || DECL_ARTIFICIAL (field)) + continue; + + unsigned j; + tree name = DECL_NAME (field); + /* Skip if field is anonymous. */ + if (!name || !identifier_p (name)) + continue; + + auto_vec base_vardecls; + tree binfo; + tree base_binfo; + /* Iterate through all of the base classes looking for possibly + shadowed non-static data members. */ + for (binfo = TYPE_BINFO (t), j = 0; + BINFO_BASE_ITERATE (binfo, j, base_binfo); j++) + { + tree basetype = BINFO_TYPE (base_binfo); + tree candidate = lookup_field (basetype, name, + /* protect */ 2, + /* want_type */ 0, + /* once_suffices */ true); + if (candidate) + { + /* If we went up the hierarchy to a base class with multiple + inheritance, there could be multiple matches in which case + a TREE_LIST is returned. */ + if (TREE_TYPE (candidate) == error_mark_node) + { + for (; candidate; candidate = TREE_CHAIN (candidate)) + { + tree candidate_field = TREE_VALUE (candidate); + tree candidate_klass = DECL_CONTEXT (candidate_field); + if (accessible_base_p (t, candidate_klass, true)) + base_vardecls.safe_push (candidate_field); + } + } + else if (accessible_base_p (t, DECL_CONTEXT (candidate), true)) + base_vardecls.safe_push (candidate); + } + } + + /* Field was not found among the base classes. */ + if (base_vardecls.is_empty ()) + continue; + + /* Emit a warning for each field similarly named found + in the base class hierarchy. */ + for (tree base_vardecl : base_vardecls) + { + if (base_vardecl) + { + auto_diagnostic_group d; + if (warning_at (location_of (field), OPT_Wshadow, + "%qD might shadow %qD", field, base_vardecl)) + inform (location_of (base_vardecl), + " %qD name already in use here", base_vardecl); + } + } + } +} + /* Recursive helper for finish_struct_anon. */ static void @@ -7654,6 +7727,8 @@ finish_struct_1 (tree t) if (warn_overloaded_virtual) warn_hidden (t); + if (warn_shadow) + warn_name_hiding (t); /* Class layout, assignment of virtual table slots, etc., is now complete. Give the back end a chance to tweak the visibility of diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 3ca011c61c8..890326f0fd8 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7554,11 +7554,13 @@ extern tree lookup_base (tree, tree, base_access, extern tree dcast_base_hint (tree, tree); extern int accessible_p (tree, tree, bool); extern int accessible_in_template_p (tree, tree); -extern tree lookup_field (tree, tree, int, bool); +extern tree lookup_field (tree, tree, int, bool, + bool once_suffices = false); extern tree lookup_fnfields (tree, tree, int, tsubst_flags_t); extern tree lookup_member (tree, tree, int, bool, tsubst_flags_t, - access_failure_info *afi = NULL); + access_failure_info *afi = NULL, + bool once_suffices = false); extern tree lookup_member_fuzzy (tree, tree, bool); extern tree locate_field_accessor (tree, tree, bool); extern int look_for_overrides (tree, tree); @@ -7577,7 +7579,8 @@ extern tree binfo_for_vbase (tree, tree); extern tree look_for_overrides_here (tree, tree); #define dfs_skip_bases ((tree)1) extern tree dfs_walk_all (tree, tree (*) (tree, void *), - tree (*) (tree, void *), void *); + tree (*) (tree, void *), void *, + bool stop_on_success = false); extern tree dfs_walk_once (tree, tree (*) (tree, void *), tree (*) (tree, void *), void *); extern tree binfo_via_virtual (tree, tree); diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc index cd80f285ac9..32bd35f3744 100644 --- a/gcc/cp/search.cc +++ b/gcc/cp/search.cc @@ -1145,13 +1145,19 @@ build_baselink (tree binfo, tree access_binfo, tree functions, tree optype) WANT_TYPE is 1 when we should only return TYPE_DECLs. + ONCE_SUFFICES is 1 when we should return upon first find of + the member in a branch of the inheritance hierarchy tree, rather + than collect all similarly named members further in that branch. + Does not impede other parallel branches of the tree. + If nothing can be found return NULL_TREE and do not issue an error. If non-NULL, failure information is written back to AFI. */ tree lookup_member (tree xbasetype, tree name, int protect, bool want_type, - tsubst_flags_t complain, access_failure_info *afi /* = NULL */) + tsubst_flags_t complain, access_failure_info *afi /* = NULL */, + bool once_suffices /* = false */) { tree rval, rval_binfo = NULL_TREE; tree type = NULL_TREE, basetype_path = NULL_TREE; @@ -1202,7 +1208,7 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type, lfi.type = type; lfi.name = name; lfi.want_type = want_type; - dfs_walk_all (basetype_path, &lookup_field_r, NULL, &lfi); + dfs_walk_all (basetype_path, &lookup_field_r, NULL, &lfi, once_suffices); rval = lfi.rval; rval_binfo = lfi.rval_binfo; if (rval_binfo) @@ -1392,10 +1398,12 @@ lookup_member_fuzzy (tree xbasetype, tree name, bool want_type_p) return NULL_TREE. */ tree -lookup_field (tree xbasetype, tree name, int protect, bool want_type) +lookup_field (tree xbasetype, tree name, int protect, bool want_type, + bool once_suffices /* = false */) { tree rval = lookup_member (xbasetype, name, protect, want_type, - tf_warning_or_error); + tf_warning_or_error, /* afi */ NULL, + once_suffices); /* Ignore functions, but propagate the ambiguity list. */ if (!error_operand_p (rval) @@ -1480,11 +1488,14 @@ adjust_result_of_qualified_name_lookup (tree decl, of PRE_FN and POST_FN can be NULL. At each node, PRE_FN and POST_FN are passed the binfo to examine and the caller's DATA value. All paths are walked, thus virtual and morally virtual - binfos can be multiply walked. */ + binfos can be multiply walked. + If STOP_ON_SUCCESS is 1, do not walk deeper the current hierarchy + tree once PRE_FN returns non-NULL. */ tree dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *), - tree (*post_fn) (tree, void *), void *data) + tree (*post_fn) (tree, void *), void *data, + bool stop_on_success /* = false */) { tree rval; unsigned ix; @@ -1496,7 +1507,7 @@ dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *), rval = pre_fn (binfo, data); if (rval) { - if (rval == dfs_skip_bases) + if (rval == dfs_skip_bases || stop_on_success) goto skip_bases; return rval; } @@ -1505,7 +1516,8 @@ dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *), /* Find the next child binfo to walk. */ for (ix = 0; BINFO_BASE_ITERATE (binfo, ix, base_binfo); ix++) { - rval = dfs_walk_all (base_binfo, pre_fn, post_fn, data); + rval = dfs_walk_all (base_binfo, pre_fn, post_fn, + data, stop_on_success); if (rval) return rval; } diff --git a/gcc/testsuite/g++.dg/pr12341-1.C b/gcc/testsuite/g++.dg/pr12341-1.C new file mode 100644 index 00000000000..d66b77a921d --- /dev/null +++ b/gcc/testsuite/g++.dg/pr12341-1.C @@ -0,0 +1,65 @@ +// PR c++/12341 +/* { dg-do compile } */ +/* { dg-additional-options "-Wshadow" } */ + +class A { +protected: + int aaa; /* { dg-line A_def_aaa } */ +}; + +// Check simple inheritance is checked for. +class B : public A { +public: + int aaa; /* { dg-line B_shadowing_aaa } */ + /* { dg-warning "'B::aaa' might shadow 'A::aaa'" "" { target *-*-* } .-1 } */ + /* { dg-note "'A::aaa' name already in use here" "" { target *-*-* } A_def_aaa } */ +private: + int bbb; /* { dg-line B_def_bbb } */ + int eee; /* { dg-line B_def_eee } */ +}; + +// Check that visibility of base classes's fields is of no concern. +class C : public B { +public: + int bbb; /* { dg-warning "'C::bbb' might shadow 'B::bbb'" } */ + /* { dg-note "'B::bbb' name already in use here" "" { target *-*-* } B_def_bbb } */ +}; + +class D { +protected: + int bbb; /* { dg-line D_def_bbb } */ + int ddd; /* { dg-line D_def_ddd } */ +}; + +class E : protected D { +private: + int eee; /* { dg-line E_def_eee } */ +}; + +// all first-level base classes must be considered. +class Bi : protected B, public E { +protected: + /* Check that we stop on first ambiguous match, + that we don't walk the hierarchy deeper. */ + int aaa; /* { dg_bogus "'Bi::aaa' might shadow 'A::aaa'" } */ + /* { dg-warning "'Bi::aaa' might shadow 'B::aaa'" "" { target *-*-* } .-1 } */ + /* { dg-note "'B::aaa' name already in use here" "" { target *-*-* } B_shadowing_aaa } */ + + // All branches of a multiple inheritance tree must be explored. + int bbb; /* { dg-warning "'Bi::bbb' might shadow 'D::bbb'" } */ + /* { dg-note "'D::bbb' name already in use here" "" { target *-*-* } D_def_bbb } */ + /* { dg-warning "'Bi::bbb' might shadow 'B::bbb'" "" { target *-*-* } .-2 } */ + + // Must continue beyond the immediate parent, even in the case of multiple inheritance. + int ddd; /* { dg-warning "'Bi::ddd' might shadow 'D::ddd'" } */ + /* { dg-note "'D::ddd' name already in use here" "" { target *-*-* } D_def_ddd } */ +}; + +class BiD : public Bi { + /* If the base class does not cause a warning, + then look into each base classes of the parent's multiple inheritance */ + int eee; /* { dg-warning "'BiD::eee' might shadow 'B::eee'" } */ + /* { dg-note "'B::eee' name already in use here" "" { target *-*-* } B_def_eee } */ + /* { dg-warning "'BiD::eee' might shadow 'E::eee'" "" { target *-*-* } .-2 } */ + /* { dg-note "'E::eee' name already in use here" "" { target *-*-* } E_def_eee } */ +}; diff --git a/gcc/testsuite/g++.dg/pr12341-2.C b/gcc/testsuite/g++.dg/pr12341-2.C new file mode 100644 index 00000000000..2836ff780e5 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr12341-2.C @@ -0,0 +1,34 @@ +// PR c++/12341 test anonymous bit field +/* { dg-do compile } */ +/* { dg-additional-options "-Wshadow" } */ + +class B { +protected: + unsigned int x : 5; + unsigned int : 2; + unsigned int y : 1; + + union { + float uuu; + double vvv; + }; +}; + +// Check that anonymous bit fields do not cause spurious warnings +class D : private B { +protected: + unsigned int x : 4; /* { dg-warning "'D::x' might shadow 'B::x'" } */ + unsigned int : 4; // If test for excess errors fails, it might be from here. + int uuu; /* { dg-warning "'D::uuu' might shadow 'B::::uuu'" } */ +}; + +class E : public D { +public: + /* Do not warn if inheritance is not visible to the class, + as there is no risk even with polymorphism to mistake the fields. */ + unsigned int y; /* { dg-bogus "'E::y' might shadow 'B::y'" } */ + unsigned int vvv; /* { dg-bogus "'E::vvv' might shadow 'B::::vvv'" } */ + + // Do warn even if the type differs. Should be trivial, still test for it. + double x; /* { dg-warning "'E::x' might shadow 'D::x'" } */ +};