From patchwork Tue May 5 19:43:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 1283883 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=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org 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=bG7F07Dg; dkim-atps=neutral Received: from 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49Gqt04hDrz9sSd for ; Wed, 6 May 2020 05:44:07 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 17D4A389040B; Tue, 5 May 2020 19:44:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 17D4A389040B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1588707845; bh=rZQ++NON5bGptQlGcBWd4N/WsbfnRrcRKTqhgPvN2lc=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=bG7F07DgR8Wwgu+oEt/ShOPwHsCt7tOyEx/2vO3DIC/xR9OY78KrQe0iO+7B667V2 6XzLatF3+oZaTc0THw19ScSC3sX5bOAmTnd6b+xa0rIWpbZwedzfzcWZktySaX7ZYr Dwim22F4hg3ut3YInKAI6sXpbU+Rw1lUyuOz3830= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 5E5B6388F046 for ; Tue, 5 May 2020 19:44:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5E5B6388F046 Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-305-i4unuaGdPhefTQUTsNgpqA-1; Tue, 05 May 2020 15:43:57 -0400 X-MC-Unique: i4unuaGdPhefTQUTsNgpqA-1 Received: by mail-qt1-f199.google.com with SMTP id v18so2875821qtq.22 for ; Tue, 05 May 2020 12:43:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=ejz6jjIzGuGYm2HOTAp12yha4RHHRiYU4ip3KnRE28c=; b=cdYrrvYcZrYbDrTlBKRmgFdH5EQtPL8UKw5gIEiAypvJ2iNWZ8cE0Z7T1JR1RVDPra D8lxlosh+a26lh63kYh6k7AMyiUQFhEC5uRsX5cT6RhG7Ms6SbDmDDoFLVDbCsK4cJJ7 a641IA3rdRZoi3ewa9uGSWfTS1Tpv2VtB5IBv0v7lNqzefrsdkJSpx0FZ7eVWQ2nBbBT 81CiIEgythbFwehu4bcmT6qNhl3XjyaZZXRv/3H9wvWqELndHLDBAIpOfp7vw4YHivLc 3fWeqlG54mvdoFcfeM5eygcakpQ0Sec5d8wAgfZxwdvkSuallMRZYSkF9HXdq184hYmg 1oaw== X-Gm-Message-State: AGi0PuY0OuuuCpPycrcrh4F6oER8BM1ODYzEZTd/6/jQ80WxLN85Ns1W w8eQAQAU2HzJZKqS4QNT3jlRrZJLLLYa9LyjgJNB/bdYrAzJfKggxAgoZA/kSNi88UMzEv+Hyf5 aruz79uDtcfmu1jYPWg== X-Received: by 2002:a05:620a:13b2:: with SMTP id m18mr4996112qki.459.1588707836249; Tue, 05 May 2020 12:43:56 -0700 (PDT) X-Google-Smtp-Source: APiQypKKVmsrdck9qzri927ebLXuH2froX2QRqZc6C6fN/ZqPeQR1ZBJU1yG6IxYfhehirJC2mzaRw== X-Received: by 2002:a05:620a:13b2:: with SMTP id m18mr4996086qki.459.1588707835777; Tue, 05 May 2020 12:43:55 -0700 (PDT) Received: from localhost.localdomain (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id q17sm2512065qkq.111.2020.05.05.12.43.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 May 2020 12:43:55 -0700 (PDT) To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: template instantiation during fold_for_warn [PR94038] Date: Tue, 5 May 2020 15:43:53 -0400 Message-Id: <20200505194353.2254588-1-ppalka@redhat.com> X-Mailer: git-send-email 2.26.2.447.gd61d20c9b4 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-20.5 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_H2, 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: Patrick Palka via Gcc-patches From: Patrick Palka Reply-To: Patrick Palka Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Unfortunately, the previous fix to PR94038 is fragile. When the argument to fold_for_warn is a bare CALL_EXPR, then all is well: the result of maybe_constant_value from fold_for_warn (with uid_sensitive=true) is reused via the cv_cache in the subsequent call to maybe_constant_value from cp_fold (with uid_sensitive=false), so we avoid instantiating bar. But when the argument to fold_for_warn is more complex, e.g. an INDIRECT_REF of a CALL_EXPR, as in the testcase below (due to bar() returning const int& which we need to decay to int) then from fold_for_warn we call maybe_constant_value on the INDIRECT_REF, and from cp_fold we call it on the CALL_EXPR, so there is no reuse via the cv_cache and we therefore end up instantiating bar. So for a more robust solution to this general issue of warning flags affecting code generation, it seems that we need a way to globally avoid template instantiation during constexpr evaluation whenever we're performing warning-dependent folding. To that end, this patch replaces the flag constexpr_ctx::uid_sensitive with a global flag uid_sensitive_constexpr_evaluation_p, and enables it during fold_for_warn using an RAII helper. The patch also adds a counter that keeps track of the number of times uid_sensitive_constexpr_evaluation_p is called, and we use this to determine whether the result of constexpr evaluation depended on the flag's value. This lets us safely update the cv_cache and fold_cache from fold_for_warn in the most common case where the flag's value was irrelevant during constexpr evaluation. Passes 'make check-c++' so far, and also verified that the testcase for PR93978 now successfully compiles with -O -Wall. Does this seem like the right approach? gcc/cp/ChangeLog: PR c++/94038 * constexpr.c (constexpr_ctx::uid_sensitive): Remove field. (uid_sensitive_constexpr_evaluation::value): Define. (uid_sensitive_constexpr_evaluation::counter): Define. (uid_sensitive_constexpr_evaluation_p): Define. (get_fundef_copy): Remove 'ctx' parameter. Use u_s_c_e_p instead of constexpr_ctx::uid_sensitive. (cxx_eval_call_expression): Use u_s_c_e_p instead, and test it last. Adjust call to get_fundef_copy. (instantiate_cx_fn_r): Test u_s_c_e_p so that we increment the counter. (cxx_eval_outermost_constant_expr): Remove 'uid_sensitive' parameter. Adjust function body accordingly. Use uid_sensitive_constexpr_evaluation::value. (maybe_constant_value): Remove 'uid_sensitive' parameter and adjust function body accordingly. Set up a uid_sensitive_constexpr_evaluation_marker, and use it to conditionally update the cv_cache. * cp-gimplify.h (cp_fold): Set up a uid_sensitive_constexpr_evaluation_marker, and use it to conditionally update the fold_cache. * cp-tree.h (maybe_constant_value): Update declaration. (uid_sensitive_constexpr_evaluation_p): Declare. (struct uid_sensitive_constexpr_evaluation): Define. (struct sensitive_constexpr_evaluation_marker): Define. * expr.c (fold_for_warn): Enable uid-sensitive constexpr evaluation before calling the folding subroutines. Drop all but the first argument to maybe_constant_value. gcc/testsuite/ChangeLog: PR c++/94038 * g++.dg/warn/pr94038-2.C: New test. --- gcc/cp/constexpr.c | 60 +++++++++++++++++---------- gcc/cp/cp-gimplify.c | 13 ++++-- gcc/cp/cp-tree.h | 44 +++++++++++++++++++- gcc/cp/expr.c | 4 +- gcc/testsuite/g++.dg/warn/pr94038-2.C | 28 +++++++++++++ 5 files changed, 120 insertions(+), 29 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/pr94038-2.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 637cb746576..7bce9e6e9b0 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1089,11 +1089,28 @@ struct constexpr_ctx { bool strict; /* Whether __builtin_is_constant_evaluated () should be true. */ bool manifestly_const_eval; - /* Whether we want to avoid doing anything that will cause extra DECL_UID - generation. */ - bool uid_sensitive; }; +/* This internal flag controls whether we should avoid doing anything during + constexpr evaluation that would cause extra DECL_UID generation, such as + template instantiation and function copying. */ + +bool uid_sensitive_constexpr_evaluation::value; + +/* An internal counter that keeps track of the number of times + uid_sensitive_constexpr_evaluation_p was called. */ + +unsigned uid_sensitive_constexpr_evaluation::counter; + +/* The public accessor for uid_sensitive_constexpr_evaluation::value. */ + +bool +uid_sensitive_constexpr_evaluation_p () +{ + ++uid_sensitive_constexpr_evaluation::counter; + return uid_sensitive_constexpr_evaluation::value; +} + /* A table of all constexpr calls that have been evaluated by the compiler in this translation unit. */ @@ -1156,7 +1173,7 @@ static GTY(()) hash_map *fundef_copies_table; is parms, TYPE is result. */ static tree -get_fundef_copy (const constexpr_ctx *ctx, constexpr_fundef *fundef) +get_fundef_copy (constexpr_fundef *fundef) { tree copy; bool existed; @@ -1173,7 +1190,7 @@ get_fundef_copy (const constexpr_ctx *ctx, constexpr_fundef *fundef) } else if (*slot == NULL_TREE) { - if (ctx->uid_sensitive) + if (uid_sensitive_constexpr_evaluation_p ()) return NULL_TREE; /* We've already used the function itself, so make a copy. */ @@ -2292,8 +2309,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, /* We can't defer instantiating the function any longer. */ if (!DECL_INITIAL (fun) - && !ctx->uid_sensitive - && DECL_TEMPLOID_INSTANTIATION (fun)) + && DECL_TEMPLOID_INSTANTIATION (fun) + && !uid_sensitive_constexpr_evaluation_p ()) { location_t save_loc = input_location; input_location = loc; @@ -2454,7 +2471,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, gcc_assert (at_eof >= 2 && ctx->quiet); *non_constant_p = true; } - else if (tree copy = get_fundef_copy (ctx, new_call.fundef)) + else if (tree copy = get_fundef_copy (new_call.fundef)) { tree body, parms, res; releasing_vec ctors; @@ -6485,7 +6502,8 @@ instantiate_cx_fn_r (tree *tp, int *walk_subtrees, void */*data*/) && DECL_DECLARED_CONSTEXPR_P (*tp) && !DECL_INITIAL (*tp) && !trivial_fn_p (*tp) - && DECL_TEMPLOID_INSTANTIATION (*tp)) + && DECL_TEMPLOID_INSTANTIATION (*tp) + && !uid_sensitive_constexpr_evaluation_p ()) { ++function_depth; instantiate_decl (*tp, /*defer_ok*/false, /*expl_inst*/false); @@ -6552,8 +6570,7 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, bool strict = true, bool manifestly_const_eval = false, bool constexpr_dtor = false, - tree object = NULL_TREE, - bool uid_sensitive = false) + tree object = NULL_TREE) { auto_timevar time (TV_CONSTEXPR); @@ -6569,8 +6586,7 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, constexpr_global_ctx global_ctx; constexpr_ctx ctx = { &global_ctx, NULL, NULL, NULL, NULL, NULL, NULL, allow_non_constant, strict, - manifestly_const_eval || !allow_non_constant, - uid_sensitive }; + manifestly_const_eval || !allow_non_constant }; tree type = initialized_type (t); tree r = t; @@ -6660,7 +6676,7 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, auto_vec cleanups; global_ctx.cleanups = &cleanups; - if (!uid_sensitive) + if (!uid_sensitive_constexpr_evaluation::value) instantiate_constexpr_fns (r); r = cxx_eval_constant_expression (&ctx, r, false, &non_constant_p, &overflow_p); @@ -6909,14 +6925,12 @@ fold_simple (tree t) Otherwise, if T does not have TREE_CONSTANT set, returns T. Otherwise, returns a version of T without TREE_CONSTANT. MANIFESTLY_CONST_EVAL is true if T is manifestly const-evaluated - as per P0595. UID_SENSITIVE is true if we can't do anything that - would affect DECL_UID ordering. */ + as per P0595. */ static GTY((deletable)) hash_map *cv_cache; tree -maybe_constant_value (tree t, tree decl, bool manifestly_const_eval, - bool uid_sensitive) +maybe_constant_value (tree t, tree decl, bool manifestly_const_eval) { tree r; @@ -6934,8 +6948,7 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval, return t; if (manifestly_const_eval) - return cxx_eval_outermost_constant_expr (t, true, true, true, false, - decl, uid_sensitive); + return cxx_eval_outermost_constant_expr (t, true, true, true, false, decl); if (cv_cache == NULL) cv_cache = hash_map::create_ggc (101); @@ -6950,14 +6963,15 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval, return r; } - r = cxx_eval_outermost_constant_expr (t, true, true, false, false, - decl, uid_sensitive); + uid_sensitive_constexpr_evaluation_marker m; + r = cxx_eval_outermost_constant_expr (t, true, true, false, false, decl); gcc_checking_assert (r == t || CONVERT_EXPR_P (t) || TREE_CODE (t) == VIEW_CONVERT_EXPR || (TREE_CONSTANT (t) && !TREE_CONSTANT (r)) || !cp_tree_equal (r, t)); - cv_cache->put (t, r); + if (!m.changed_p ()) + cv_cache->put (t, r); return r; } diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index fc26a85f43a..2f04b41b582 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -2443,6 +2443,8 @@ cp_fold (tree x) if (tree *cached = fold_cache->get (x)) return *cached; + uid_sensitive_constexpr_evaluation_marker m; + code = TREE_CODE (x); switch (code) { @@ -2925,10 +2927,13 @@ cp_fold (tree x) return org_x; } - fold_cache->put (org_x, x); - /* Prevent that we try to fold an already folded result again. */ - if (x != org_x) - fold_cache->put (x, x); + if (!m.changed_p ()) + { + fold_cache->put (org_x, x); + /* Prevent that we try to fold an already folded result again. */ + if (x != org_x) + fold_cache->put (x, x); + } return x; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index f4328403bc7..784b6057080 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7949,7 +7949,7 @@ extern bool require_potential_rvalue_constant_expression (tree); extern tree cxx_constant_value (tree, tree = NULL_TREE); extern void cxx_constant_dtor (tree, tree); extern tree cxx_constant_init (tree, tree = NULL_TREE); -extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false, bool = false); +extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false); extern tree maybe_constant_init (tree, tree = NULL_TREE, bool = false); extern tree fold_non_dependent_expr (tree, tsubst_flags_t = tf_warning_or_error, @@ -7968,6 +7968,48 @@ extern tree fold_sizeof_expr (tree); extern void clear_cv_and_fold_caches (bool = true); extern tree unshare_constructor (tree CXX_MEM_STAT_INFO); +extern bool uid_sensitive_constexpr_evaluation_p (); + +/* An RAII sentinel to temporarily have uid_sensitive_constexpr_evaluation_p + return true. */ + +struct uid_sensitive_constexpr_evaluation +{ + const bool saved_value; + uid_sensitive_constexpr_evaluation () + : saved_value (value) + { + value = true; + } + + ~uid_sensitive_constexpr_evaluation () + { + value = saved_value; + } + + static bool value; + static unsigned counter; +}; + +/* A class used to determine whether uid_sensitive_constexpr_evaluation_p was + called within some scope. We use this to avoid updating fold_cache and + cv_cache when the result of constexpr evaluation depended on the value of + uid_sensitive_constexpr_evaluation_p. */ + +struct uid_sensitive_constexpr_evaluation_marker +{ + const unsigned saved_counter; + uid_sensitive_constexpr_evaluation_marker () + : saved_counter (uid_sensitive_constexpr_evaluation::counter) + { + } + + bool changed_p () + { + return uid_sensitive_constexpr_evaluation::counter != saved_counter; + } +}; + /* In cp-ubsan.c */ extern void cp_ubsan_maybe_instrument_member_call (tree); extern void cp_ubsan_instrument_member_accesses (tree *); diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c index 9b535708c57..97671edf800 100644 --- a/gcc/cp/expr.c +++ b/gcc/cp/expr.c @@ -399,6 +399,8 @@ fold_for_warn (tree x) { /* C++ implementation. */ + uid_sensitive_constexpr_evaluation s; + /* It's not generally safe to fully fold inside of a template, so call fold_non_dependent_expr instead. */ if (processing_template_decl) @@ -410,7 +412,7 @@ fold_for_warn (tree x) return f; } else if (cxx_dialect >= cxx11) - x = maybe_constant_value (x, NULL_TREE, false, true); + x = maybe_constant_value (x); return c_fully_fold (x, /*for_init*/false, /*maybe_constp*/NULL); } diff --git a/gcc/testsuite/g++.dg/warn/pr94038-2.C b/gcc/testsuite/g++.dg/warn/pr94038-2.C new file mode 100644 index 00000000000..a468cc055eb --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr94038-2.C @@ -0,0 +1,28 @@ +// PR c++/94038 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-O -Wall" } + +static constexpr int x = 0; + +template +constexpr const int& +foo() +{ + static_assert(T(1) == 0, ""); + return x; +} + +template +constexpr const int& +bar() +{ + return foo(); +} + +constexpr int +baz(int a) +{ + return a; +} + +static_assert(decltype(baz(bar())){} == 0, "");