From patchwork Wed Dec 20 22:07:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1878726 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=BXavg4n9; 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 4SwSP958cCz1ydZ for ; Thu, 21 Dec 2023 09:08:12 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B7DC23861821 for ; Wed, 20 Dec 2023 22:08:09 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 5213C3858CD1 for ; Wed, 20 Dec 2023 22:07:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5213C3858CD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5213C3858CD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703110078; cv=none; b=te7e7O6bESPgloVVKODuCy2yE2VINHrueS9XBdGZmJhAPCespS84+vSh0CWhdTQDWQ9azZ5yzMxD9q8nzSs+JQlO//Wwu/eFKzNkZrK19vLAR8Hug1tO/zWzP1qglluwvOjBGiLmrYP6EiDSvlj38tbAYaRiav6FrTZOPT75iHg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703110078; c=relaxed/simple; bh=COk4mrnELu7ihnxMAzC3Wi9s23W+qtrAWHSZ7hDT4yo=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=s2Wy2GMb0+R51nupQ53JOh+RPk057D/m3LBGucMIelqUBXU610r0b20qs2wbX82qjT4loLmLbPLKR64byl/yPT7CCSCU+A4FKf45JXu9kR6mBx0EJghh9ZYuEEC8tcQPre2+7EZS2nFm0S6pQt9nkcpXqP08PP/BXzsozTALHIQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1703110076; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kyOrYiCs5+usG9wk4aQUSene3lV/HPWksk2MvteHhXc=; b=BXavg4n9t3+ZGC+YG/hjt2pshRc0bKEMEIXvZKwEfFHka51wD06/3a3C9fBGP23YB8hsqa O+HviCfbg/0wA7No2vAOdlTaSFKG+i6DsMH9MRDpGH04GBKu27tbBbFaWqfn2f9iCCoVol oPkJjBJlitwrut29ppX+sqJgDsKUn5g= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-650-8X9TCeL6Psqo1SAwkAuEIQ-1; Wed, 20 Dec 2023 17:07:54 -0500 X-MC-Unique: 8X9TCeL6Psqo1SAwkAuEIQ-1 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-67f3d90abfdso1125136d6.2 for ; Wed, 20 Dec 2023 14:07:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703110072; x=1703714872; 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=kyOrYiCs5+usG9wk4aQUSene3lV/HPWksk2MvteHhXc=; b=t4c0w472WH81nrCHWoXCd47N9HwCxBX0kUhvKIeGxAkrMrLojYzI8Auab8Qy0W/T7k 3SOOb08sQ6QiaYuwoUTwHKQn0C3LwthhBoHXje/X2Xsm9anLUE9AHEOCaxAkiJ23cXOo 00WFNguzQIkO2KP4+CfpRVbKGRkc3duHLB9djlViLPLTIw7ezSk9jz6/8Oyi8M7gPCLy Cu/Hgs3N3FPS9J3PkfwKbisu/2KTFp5vOOTp0vuiiXMqbs9hJjwH1vuGLeEXo+tbNZJo x2Z7E7nhLlw5hoXJuaWPQb7O+xMAtmDXn8teqznvP0Bch57ISXr3AsiMHtjID5DHLf3l ubZQ== X-Gm-Message-State: AOJu0YwMUm/9dnlmF9za3/mv2KQp2o2mCAMxVqvUuOm/i4WniqvdoC8t AKWf5XAd+xnC3C9MDdC5juQqw0ek2FMIlYeZeaA/yn2s1yUHVFVW2PHfcOINp4wDOSylQYyuJvV CVW9BYWFv5/byx/p/hTMvs3sZ0EPpriiXlzHZ1F30+Jjd7oDgQXRbEUeVOcnw3nTX8VswHjBrXX I= X-Received: by 2002:ad4:5961:0:b0:67f:43f8:cc6f with SMTP id eq1-20020ad45961000000b0067f43f8cc6fmr7727431qvb.61.1703110072558; Wed, 20 Dec 2023 14:07:52 -0800 (PST) X-Google-Smtp-Source: AGHT+IGEIs07+bABuSv+2Xe1+YM35S0waY3pFnt/HxmYTVMwsatxz3Xi8R2RBN3URSg0mriv4EnNgQ== X-Received: by 2002:ad4:5961:0:b0:67f:43f8:cc6f with SMTP id eq1-20020ad45961000000b0067f43f8cc6fmr7727413qvb.61.1703110072143; Wed, 20 Dec 2023 14:07:52 -0800 (PST) Received: from localhost.localdomain (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id x6-20020a0cfe06000000b0067f72373b3fsm190762qvr.130.2023.12.20.14.07.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Dec 2023 14:07:51 -0800 (PST) From: Patrick Palka To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com, Patrick Palka Subject: [PATCH] c++: fix -Wparentheses with boolean-like class types Date: Wed, 20 Dec 2023 17:07:49 -0500 Message-ID: <20231220220749.632100-1-ppalka@redhat.com> X-Mailer: git-send-email 2.43.0.121.g624eb90fa8 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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 Bootstrap and regtesting in progress on x86_64-pc-linux-gnu, does this look OK for trunk if successful? -- >8 -- Since r14-4977-g0f2e2080685e75 the -Wparentheses warning now undesirably warns on the idiom Wparentheses-34.C:9:14: warning: suggest parentheses around assignment used as truth value [-Wparentheses] 9 | b = v[i] = true; | ^~~~ where v has type std::vector. That commit intended to only extend the existing diagnostics so that they happen in a template context as well, but the refactoring of is_assignment_op_expr_p caused us for this particular -Wparentheses warning (from convert_for_assignment) to now consider user-defined operator= instead of just built-in operator=. And since std::vector is really a bitset, whose operator[] returns a class type with such a user-defined operator= (taking bool), we now warn. But arguably "boolish" class types should be treated like ordinary bool as far as the warning is concerned. To that end this patch relaxes the warning for such types, specifically when the (class) type can be (implicitly) converted to a and assigned from a bool. This should cover at least implementations of std::vector::reference. gcc/cp/ChangeLog: * cp-tree.h (maybe_warn_unparenthesized_assignment): Add 'nested_p' bool parameter. * semantics.cc (is_assignment_op_expr_p): Add 'rhs' bool parameter and set it accordingly. (class_type_is_boolish_cache): Define. (class_type_is_boolish): Define. (maybe_warn_unparenthesized_assignment): Add 'nested_p' bool parameter. Relax the warning for nested assignments to boolean-like class types. (maybe_convert_cond): Pass nested_p=false to maybe_warn_unparenthesized_assignment. * typeck.cc (convert_for_assignment): Pass nested_p=true to maybe_warn_unparenthesized_assignment. gcc/testsuite/ChangeLog: * g++.dg/warn/Wparentheses-34.C: New test. --- gcc/cp/cp-tree.h | 2 +- gcc/cp/semantics.cc | 106 ++++++++++++++++++-- gcc/cp/typeck.cc | 2 +- gcc/testsuite/g++.dg/warn/Wparentheses-34.C | 31 ++++++ 4 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-34.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 1979572c365..97065cccf3d 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7928,7 +7928,7 @@ extern tree lambda_regenerating_args (tree); extern tree most_general_lambda (tree); extern tree finish_omp_target (location_t, tree, tree, bool); extern void finish_omp_target_clauses (location_t, tree, tree *); -extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t); +extern void maybe_warn_unparenthesized_assignment (tree, bool, tsubst_flags_t); extern tree cp_check_pragma_unroll (location_t, tree); /* in tree.cc */ diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 64839b1ac87..92acd560fa4 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -839,10 +839,11 @@ finish_goto_stmt (tree destination) return add_stmt (build_stmt (input_location, GOTO_EXPR, destination)); } -/* Returns true if T corresponds to an assignment operator expression. */ +/* Returns true if T corresponds to an assignment operator expression, + and sets *LHS to its left-hand-side operand if so. */ static bool -is_assignment_op_expr_p (tree t) +is_assignment_op_expr_p (tree t, tree *lhs) { if (t == NULL_TREE) return false; @@ -850,7 +851,10 @@ is_assignment_op_expr_p (tree t) if (TREE_CODE (t) == MODIFY_EXPR || (TREE_CODE (t) == MODOP_EXPR && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR)) - return true; + { + *lhs = TREE_OPERAND (t, 0); + return true; + } tree call = extract_call_expr (t); if (call == NULL_TREE @@ -859,26 +863,107 @@ is_assignment_op_expr_p (tree t) return false; tree fndecl = cp_get_callee_fndecl_nofold (call); - return fndecl != NULL_TREE - && DECL_ASSIGNMENT_OPERATOR_P (fndecl) - && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR); + if (fndecl != NULL_TREE + && DECL_ASSIGNMENT_OPERATOR_P (fndecl) + && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR)) + { + *lhs = get_nth_callarg (call, 0); + return true; + } + + return false; } +static GTY((deletable)) hash_map *class_type_is_boolish_cache; + +/* Return true if the class type TYPE can be converted to and assigned + from a boolean. */ + +static bool +class_type_is_boolish (tree type) +{ + type = TYPE_MAIN_VARIANT (type); + if (!COMPLETE_TYPE_P (type)) + return false; + + if (bool *r = hash_map_safe_get (class_type_is_boolish_cache, type)) + return *r; + + bool has_bool_conversion = false; + bool has_bool_assignment = false; + + tree ops = lookup_conversions (type); + for (; ops; ops = TREE_CHAIN (ops)) + { + tree op = TREE_VALUE (ops); + if (!DECL_NONCONVERTING_P (op) + && TREE_CODE (TREE_TYPE (DECL_NAME (op))) == BOOLEAN_TYPE) + { + has_bool_conversion = true; + break; + } + } + + if (!has_bool_conversion) + { + hash_map_safe_put (class_type_is_boolish_cache, type, false); + return false; + } + + ops = lookup_fnfields (type, assign_op_identifier, /*protect=*/0, tf_none); + for (tree op : ovl_range (BASELINK_FUNCTIONS (ops))) + { + op = STRIP_TEMPLATE (op); + if (TREE_CODE (op) != FUNCTION_DECL) + continue; + tree parm = DECL_CHAIN (DECL_ARGUMENTS (op)); + tree parm_type = non_reference (TREE_TYPE (parm)); + if (TREE_CODE (parm_type) == BOOLEAN_TYPE) + { + has_bool_assignment = true; + break; + } + } + + bool boolish = has_bool_conversion && has_bool_assignment; + hash_map_safe_put (class_type_is_boolish_cache, type, boolish); + return boolish; +} + + /* Maybe warn about an unparenthesized 'a = b' (appearing in a - boolean context where 'a == b' might have been intended). */ + boolean context where 'a == b' might have been intended). + NESTED_P is true if T appears as the RHS of another assignment. */ void -maybe_warn_unparenthesized_assignment (tree t, tsubst_flags_t complain) +maybe_warn_unparenthesized_assignment (tree t, bool nested_p, + tsubst_flags_t complain) { t = STRIP_REFERENCE_REF (t); + tree lhs; if ((complain & tf_warning) && warn_parentheses - && is_assignment_op_expr_p (t) + && is_assignment_op_expr_p (t, &lhs) /* A parenthesized expression would've had this warning suppressed by finish_parenthesized_expr. */ && !warning_suppressed_p (t, OPT_Wparentheses)) { + /* In a = b = c, don't warn if b is a boolean-like class type. */ + if (nested_p) + { + /* The caller already checked this. */ + gcc_checking_assert (TREE_CODE (TREE_TYPE (t)) != BOOLEAN_TYPE); + + if (TREE_CODE (lhs) == ADDR_EXPR) + lhs = TREE_OPERAND (lhs, 0); + + tree lhs_type = non_reference (TREE_TYPE (lhs)); + if (CLASS_TYPE_P (lhs_type) + && class_type_is_boolish (lhs_type)) + return; + } + warning_at (cp_expr_loc_or_input_loc (t), OPT_Wparentheses, "suggest parentheses around assignment used as truth value"); suppress_warning (t, OPT_Wparentheses); @@ -903,7 +988,8 @@ maybe_convert_cond (tree cond) if (warn_sequence_point && !processing_template_decl) verify_sequence_points (cond); - maybe_warn_unparenthesized_assignment (cond, tf_warning_or_error); + maybe_warn_unparenthesized_assignment (cond, /*nested_p=*/false, + tf_warning_or_error); /* Do the conversion. */ cond = convert_from_reference (cond); diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index a6e2f4ee7da..195114e716e 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10381,7 +10381,7 @@ convert_for_assignment (tree type, tree rhs, does not. */ if (TREE_CODE (type) == BOOLEAN_TYPE && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE) - maybe_warn_unparenthesized_assignment (rhs, complain); + maybe_warn_unparenthesized_assignment (rhs, /*nested_p=*/true, complain); if (complain & tf_warning) warn_for_address_of_packed_member (type, rhs); diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-34.C b/gcc/testsuite/g++.dg/warn/Wparentheses-34.C new file mode 100644 index 00000000000..2100c8a193d --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-34.C @@ -0,0 +1,31 @@ +// Verify our -Wparentheses warning handles "boolish" class types +// such as std::vector's reference type the same as ordinary +// bool. +// { dg-additional-options "-Wparentheses" } + +#include + +void f(std::vector v, int i) { + bool b; + b = v[i] = true; + b = v[i] = v[i+1]; + + if (v[i] = 42) { } // { dg-message "parentheses" } + if (v[i] = v[i+1]) { } // { dg-message "parentheses" } + + if ((v[i] = 42)) { } + if ((v[i] = v[i+1])) { } +} + +template +void ft(std::vector v, int i) { + bool b; + b = v[i] = true; + b = v[i] = v[i+1]; + + if (v[i] = 42) { } // { dg-message "parentheses" } + if (v[i] = v[i+1]) { } // { dg-message "parentheses" } + + if ((v[i] = 42)) { } + if ((v[i] = v[i+1])) { } +}