From patchwork Thu Jun 10 01:46:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 1490139 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=wcYyflbV; dkim-atps=neutral Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G0n0Z3Lxdz9sPf for ; Thu, 10 Jun 2021 11:47:25 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6E07639DC4C6 for ; Thu, 10 Jun 2021 01:47:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6E07639DC4C6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1623289642; bh=9MfGd4a2hxQmjZyqothbpTZPrZj+t8jyyVCYhe4BZl8=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=wcYyflbVZAyl++olUSm3uSbBFQAVgIvlsMwtj5caYyH4sJaeJYbd1mF+IrT1h/WY6 ZuDrPM8d3XCSlrI/OZjnVim8eB3SwRjhSzBmMYuKViVyKL8w3c7rLpFCXHKRsVO9qX xtwqeL910xH3XVzJlLQgogcaA2S7wCNwe5InDiSA= 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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 6CD6139A183B for ; Thu, 10 Jun 2021 01:46:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6CD6139A183B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-589-AmY94TnZNUq3pKf3YpjfiQ-1; Wed, 09 Jun 2021 21:46:36 -0400 X-MC-Unique: AmY94TnZNUq3pKf3YpjfiQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 39F7E100C623 for ; Thu, 10 Jun 2021 01:46:35 +0000 (UTC) Received: from pdp-11.hsd1.ma.comcast.net (ovpn-117-182.rdu2.redhat.com [10.10.117.182]) by smtp.corp.redhat.com (Postfix) with ESMTP id B71305D6BA; Thu, 10 Jun 2021 01:46:34 +0000 (UTC) To: Jason Merrill , GCC Patches Subject: [PATCH] c++: Extend std::is_constant_evaluated in if warning [PR100995] Date: Wed, 9 Jun 2021 21:46:28 -0400 Message-Id: <20210610014628.2194915-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.3 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_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Jakub pointed me at which shows that our existing warning could be extended to handle more cases. This patch implements that. A minor annoyance was handling macros, in libstdc++ we have reference operator[](size_type __pos) { __glibcxx_assert(__pos <= size()); ... } wherein __glibcxx_assert expands to if (__builtin_is_constant_evaluated() && !bool(__pos <= size()) ... but I'm of a mind to not warn on that. Possible tweaks: merge the "always true" warnings and say something about a manifestly evaluated context, and perhaps add an early exit for TREE_CONSTANT trees because for constexpr if we are going to call maybe_warn_for_constant_evaluated twice. Once consteval if makes it in, we should tweak this warning one more time. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/100995 gcc/cp/ChangeLog: * semantics.c (find_std_constant_evaluated_r): New. (maybe_warn_for_constant_evaluated): New. (finish_if_stmt_cond): Call it. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning. * g++.dg/cpp2a/is-constant-evaluated12.C: New test. --- gcc/cp/semantics.c | 85 ++++++++++++++++--- .../g++.dg/cpp2a/is-constant-evaluated12.C | 79 +++++++++++++++++ .../g++.dg/cpp2a/is-constant-evaluated9.C | 4 +- 3 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C base-commit: 4f625f47b4456e5c05a025fca4d072831e59126c diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index f506a239864..07459a357e2 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -927,6 +927,75 @@ is_std_constant_evaluated_p (tree fn) return name && id_equal (name, "is_constant_evaluated"); } +/* Callback function for maybe_warn_for_constant_evaluated that looks + for calls to std::is_constant_evaluated in TP. */ + +static tree +find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *) +{ + tree t = *tp; + + if (TYPE_P (t) || TREE_CONSTANT (t)) + { + *walk_subtrees = false; + return NULL_TREE; + } + + switch (TREE_CODE (t)) + { + case CALL_EXPR: + if (is_std_constant_evaluated_p (t)) + return t; + break; + case EXPR_STMT: + /* Don't warn in statement expressions. */ + *walk_subtrees = false; + return NULL_TREE; + default: + break; + } + + return NULL_TREE; +} + +/* In certain contexts, std::is_constant_evaluated() is always true (for + instance, in a consteval function or in a constexpr if), or always false + (e.g., in a non-constexpr non-consteval function) so give the user a clue. */ + +static void +maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if) +{ + if (!warn_tautological_compare) + return; + + /* Suppress warning for std::is_constant_evaluated if the conditional + comes from a macro. */ + if (from_macro_expansion_at (EXPR_LOCATION (cond))) + return; + + cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r, + NULL); + if (cond) + { + if (constexpr_if) + warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare, + "% always evaluates to " + "true in %"); + else if (!DECL_DECLARED_CONSTEXPR_P (current_function_decl) + /* C++17 lambda op() is implicitly constexpr but finish_function + may not have marked it as such. */ + && !(cxx_dialect >= cxx17 + && LAMBDA_TYPE_P (CP_DECL_CONTEXT (current_function_decl)))) + warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare, + "% always evaluates to " + "false in a non-% function"); + else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl)) + warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare, + "% always evaluates to " + "true in a % function"); + } +} + /* Process the COND of an if-statement, which may be given by IF_STMT. */ @@ -942,23 +1011,11 @@ finish_if_stmt_cond (tree cond, tree if_stmt) converted to bool. */ && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node) { - /* if constexpr (std::is_constant_evaluated()) is always true, - so give the user a clue. */ - if (warn_tautological_compare) - { - tree t = cond; - if (TREE_CODE (t) == CLEANUP_POINT_EXPR) - t = TREE_OPERAND (t, 0); - if (TREE_CODE (t) == CALL_EXPR - && is_std_constant_evaluated_p (t)) - warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare, - "%qs always evaluates to true in %", - "std::is_constant_evaluated"); - } - + maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true); cond = instantiate_non_dependent_expr (cond); cond = cxx_constant_value (cond, NULL_TREE); } + maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false); finish_cond (&IF_COND (if_stmt), cond); add_stmt (if_stmt); THEN_CLAUSE (if_stmt) = push_stmt_list (); diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C new file mode 100644 index 00000000000..0de6bf74f4a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C @@ -0,0 +1,79 @@ +// PR c++/100995 +// { dg-do compile { target c++20 } } +// { dg-options "-Wtautological-compare" } + +namespace std { + constexpr inline bool + is_constant_evaluated () noexcept + { + return __builtin_is_constant_evaluated (); + } +} + +template +struct X { }; + +template +struct Y { }; + +template // { dg-warning "always evaluates to true" } +struct Z { }; + +constexpr bool b = true; + +#define __glibcxx_assert(cond) \ + if (__builtin_is_constant_evaluated() && !bool(cond)) \ +__builtin_unreachable() +#define CHECK __builtin_is_constant_evaluated() // { dg-warning "always evaluates to false" } +#define CHECK2 __builtin_is_constant_evaluated() + +int +foo () +{ + if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to false" } + return 1; + __glibcxx_assert(b); + if (CHECK && b) + return 2; + if (CHECK2) + return 3; + return 0; +} + +constexpr int +bar () +{ + if (std::is_constant_evaluated ()) + return 1; + if constexpr (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" } + return 2; + if constexpr (std::is_constant_evaluated () && b) // { dg-warning "always evaluates to true" } + return 3; + if constexpr (!std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" } + return 4; + return 0; +} + +consteval int +baz () +{ + if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" } + return 1; + return 0; +} + +int +qux () +{ + if (({ static bool a = std::is_constant_evaluated (); a; })) + return 1; + if (({ bool a = std::is_constant_evaluated (); a; })) + return 2; + if (static bool a = std::is_constant_evaluated (); a) + return 3; + if (bool a = std::is_constant_evaluated (); a) + return 4; + if constexpr (constexpr bool a = std::is_constant_evaluated (); a) + return 5; + return 0; +} diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C index 0e96a1a3729..5e405e71cc0 100644 --- a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C +++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C @@ -32,7 +32,7 @@ constexpr int foo3(int i) { // I is not a constant expression but we short-circuit it. - if constexpr (__builtin_is_constant_evaluated () || i) + if constexpr (__builtin_is_constant_evaluated () || i) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." } return 42; else return i; @@ -42,7 +42,7 @@ constexpr int foo4(int i) { const int j = 0; - if constexpr (j && __builtin_is_constant_evaluated ()) + if constexpr (j && __builtin_is_constant_evaluated ()) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." } return 42; else return i;