From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) by sourceware.org (Postfix) with ESMTPS id BCA033858D1E for ; Sat, 24 Dec 2022 19:04:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BCA033858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-qt1-x835.google.com with SMTP id c11so6017566qtn.11 for ; Sat, 24 Dec 2022 11:04:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=AqRvqB8QyI+MTFQ5zEs7bxlfvim7GUTiLZaf9CCajs0=; b=E+BIYG3/Nrq8yM8vrnYYmPrRdhFXTT+kO/CSpb5zUNBpdcaZwsb19WY6Df28GnEdwm +t46zKo0ebBNxTvVtHjEa8TaGhR5kHQfFrWaNq7GV4UQb/QGPK3IeCRCIN/ujUi6D55s 1dO1EH7pW1KeePlQWeEHI4mxLy0wlII4sn7c6l4oxBc+UL5uCZ2EVw8jsNQT+Bwt6lXE wDAgn7xC3SvTqJsTpmNhsQKWv2e72QmEk04iMy9f8ueOt4oLK6/UE5MJLjv1fVJbIudE /RWVbvoEQ/wfPteun3jNRhioVGa1kZOw4EkL4LPq5XgmkM45WbLPxzUA/bz4TZcYCAbe JAXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=AqRvqB8QyI+MTFQ5zEs7bxlfvim7GUTiLZaf9CCajs0=; b=0SUAUuFp2pFMHOCZut1Nrvb/j6cBowBDZ1k5rJToXgF4s55qrb+1auHL2BR3S+ft8O wYIv1liC7mq3CWpniQBk4lDdkmUzArr392wlZM0xegUmIwspJWM3si9ikrxDpoosBmtJ fShSwF4XreeVDdOyU9jjnjnP+QQFqp9jhgnoJN5M3EcHdZRqaklHOT+S7vzaRAc4Gsiw JMhWPvR7FdWB30q+eYfQjP2yly4cMMvHlH+lRXuGUXxnCB9ZO+HlaXMIfDa7pCREt4xI LPREGJiWvJDNvBOVv1t+ucwfixp1ngEDOjelMS/vr5pnYY0o4US6CGsLRtPLbn5ZFzLB Scfw== X-Gm-Message-State: AFqh2kpB86yPzOtvyJfHPK4ufgS/qRXHnO5T1gUc2z5hQYAmQmyjpVNJ iRQPjCpE7cD5CeQYfbJtX+CgbhP44R4= X-Google-Smtp-Source: AMrXdXu64rDAzVdqWkyT5tckiX65N3xMA5kyz+lGeyAtsm5qB5qbW80PfkkTMqIfdWYMWEXQJpI65g== X-Received: by 2002:ac8:5295:0:b0:3a8:fdf:8ff8 with SMTP id s21-20020ac85295000000b003a80fdf8ff8mr16692690qtn.36.1671908640741; Sat, 24 Dec 2022 11:04:00 -0800 (PST) Received: from triangulum.attlocal.net ([2600:1700:8c20:f0b0::42]) by smtp.gmail.com with ESMTPSA id d18-20020ac847d2000000b0039cc82a319asm3815311qtr.76.2022.12.24.11.03.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 24 Dec 2022 11:04:00 -0800 (PST) From: Charlie Sale To: gcc-patches@gcc.gnu.org Cc: Charlie Sale Subject: [PATCH] cp: warn uninitialized const/ref in base class [PR80681] Date: Sat, 24 Dec 2022 14:03:43 -0500 Message-Id: <20221224190343.3490-1-softwaresale01@gmail.com> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT,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 List-Id: On this example: ``` struct Fine { private: const int f; }; struct BuggyA { const int a; int &b; }; struct BuggyB : private BuggyA { }; ``` g++ currently emits: ``` test.cc:3:19: warning: non-static const member ‘const int Fine::f’ in class without a constructor [-Wuninitialized] 3 | const int f; | ``` (relevant godbolt: https://godbolt.org/z/KGMK6e1zc) The issue here is that g++ misses the uninitialized const and ref members in BuggyA that are inherited as private in BuggyB. It should warn about those members when checking BuggyB. With this patch, g++ emits the following: ``` test.cc:3:19: warning: non-static const member ‘const int Fine::f’ in class without a constructor [-Wuninitialized] 3 | const int f; | ^ test.cc:7:19: warning: while processing ‘BuggyB’: non-static const member ‘const int BuggyA::a’ in class without a constructor [-Wuninitialized] 7 | const int a; | ^ test.cc:7:19: note: ‘BuggyB’ inherits ‘BuggyA’ as private, so all fields contained within ‘BuggyA’ are private to ‘BuggyB’ test.cc:8:14: warning: while processing ‘BuggyB’: non-static reference ‘int& BuggyA::b’ in class without a constructor [-Wuninitialized] 8 | int &b; | ^ test.cc:8:14: note: ‘BuggyB’ inherits ‘BuggyA’ as private, so all fields contained within ‘BuggyA’ are private to ‘BuggyB’ ``` Now, the compiler warns about the uninitialized members. In terms of testing, I added three tests: - a status quo test that makes sure that the existing warning behavior works - A simple test based off of the PR - Another example with multiple inheritance - A final example with mutliple levels of inheritance. These tests all pass. I also bootstrapped the project without any regressions. PR c++/80681 gcc/cp/ChangeLog: * class.cc (warn_uninitialized_const_and_ref): Extract warn logic into new func, add inform for inheritance warning (check_bases_and_members): Move warn logic to warn_unintialized_const_and_ref, check subclasses for warnings as well gcc/testsuite/ChangeLog: * g++.dg/pr80681-1.C: New test. Signed-off-by: Charlie Sale --- gcc/cp/class.cc | 110 +++++++++++++++++++++++++------ gcc/testsuite/g++.dg/pr80681-1.C | 51 ++++++++++++++ 2 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr80681-1.C diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index aebcb53739e..72172bea6ad 100644 --- a/gcc/cp/class.cc +++ b/gcc/cp/class.cc @@ -6018,6 +6018,76 @@ explain_non_literal_class (tree t) } } + +/* Warn for private const or reference class members that cannot be initialized + due to the class not having a default constructor. If a child type is + provided, then we are checking class_type's members in case they cannot be + initialized by child_type. If child_type is null, then we simply check + class_type. */ +static void +warn_uninitialized_const_and_ref (tree class_type, tree child_type) +{ + /* Check the fields on this class type. */ + tree field; + for (field = TYPE_FIELDS (class_type); field; field = DECL_CHAIN (field)) + { + /* We only want to check variable declarations. + Exclude fields that are not field decls or are not initialized. */ + if (TREE_CODE (field) != FIELD_DECL + || DECL_INITIAL (field) != NULL_TREE) + continue; + + tree type = TREE_TYPE (field); + + if (TYPE_REF_P (type)) + { + if (child_type != nullptr) + { + /* Show parent class while processing. */ + auto_diagnostic_group d; + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "while processing %qE: " + "non-static reference %q#D in class without a constructor", + child_type, field); + inform (DECL_SOURCE_LOCATION (field), + "%qE inherits %qE as private, so all fields " + "contained within %qE are private to %qE", + child_type, class_type, class_type, child_type); + } + else + { + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "non-static reference %q#D " + "in class without a constructor", field); + } + } + else if (CP_TYPE_CONST_P (type) + && (!CLASS_TYPE_P (type) + || !TYPE_HAS_DEFAULT_CONSTRUCTOR (type))) + { + if (child_type) + { + /* ditto. */ + auto_diagnostic_group d; + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "while processing %qE: " + "non-static const member %q#D in class " + "without a constructor", child_type, field); + inform (DECL_SOURCE_LOCATION (field), + "%qE inherits %qE as private, so all fields " + "contained within %qE are private to %qE", + child_type, class_type, class_type, child_type); + } + else + { + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "non-static const member %q#D " + "in class without a constructor", field); + } + } + } +} + /* Check the validity of the bases and members declared in T. Add any implicitly-generated functions (like copy-constructors and assignment operators). Compute various flag bits (like @@ -6160,30 +6230,32 @@ check_bases_and_members (tree t) initializers. */ && CLASSTYPE_NON_AGGREGATE (t)) { - tree field; + /* First, warn for this class. */ + warn_uninitialized_const_and_ref (t, nullptr /* no child class. */); - for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field)) - { - tree type; + /* Then, warn for any direct base classes. This process will not + descend all base classes, just the ones directly inherited by + this class. */ + tree binfo, base_binfo; + int idx; - if (TREE_CODE (field) != FIELD_DECL - || DECL_INITIAL (field) != NULL_TREE) - continue; + for (binfo = TYPE_BINFO (t), idx = 0; + BINFO_BASE_ITERATE (binfo, idx, base_binfo); idx++) + { + tree basetype = TREE_TYPE (base_binfo); - type = TREE_TYPE (field); - if (TYPE_REF_P (type)) - warning_at (DECL_SOURCE_LOCATION (field), - OPT_Wuninitialized, "non-static reference %q#D " - "in class without a constructor", field); - else if (CP_TYPE_CONST_P (type) - && (!CLASS_TYPE_P (type) - || !TYPE_HAS_DEFAULT_CONSTRUCTOR (type))) - warning_at (DECL_SOURCE_LOCATION (field), - OPT_Wuninitialized, "non-static const member %q#D " - "in class without a constructor", field); + /* Base types are not going to be aggregates, so don't + check for that. Otherwise, this step will never happen. */ + if (!TYPE_HAS_USER_CONSTRUCTOR (basetype) + || CLASSTYPE_NON_AGGREGATE (basetype)) + { + /* Repeat the same process on base classes, know that 't' + is a child_class of basetype. */ + warn_uninitialized_const_and_ref (basetype, t); + } } - } + } /* Synthesize any needed methods. */ add_implicitly_declared_members (t, &access_decls, cant_have_const_ctor, diff --git a/gcc/testsuite/g++.dg/pr80681-1.C b/gcc/testsuite/g++.dg/pr80681-1.C new file mode 100644 index 00000000000..40eab653aca --- /dev/null +++ b/gcc/testsuite/g++.dg/pr80681-1.C @@ -0,0 +1,51 @@ +// PR c++/80681 +// { dg-do compile } +// { dg-options "-Wuninitialized -c" } + +/* Status quo: the original should still warn correctly. */ +struct StatusQuo +{ +private: + const int a; // { dg-warning "non-static const member" } + int &b; // { dg-warning "non-static reference" } +}; + +/* Single layer of inheritance. */ +struct A { + const int a; // { dg-warning "non-static const member" } + int &b; // { dg-warning "non-static reference" } +}; + +struct B: private A { +}; + +/* multiple inheritance example. */ +struct X +{ + const int x; // { dg-warning "non-static const member" } +}; + +struct Y +{ + const int &y; // { dg-warning "non-static reference" } +}; + +struct Z : private Y, private X +{ +}; + +/* multi-level inheritance example. */ +struct M +{ + const int x; // { dg-warning "non-static const member" } +}; + +struct N: private M +{ + const int &y; // { dg-warning "non-static reference" } +}; + +struct O: private N +{ +}; + -- 2.38.1