From patchwork Thu Dec 21 17:50:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 852052 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-469730-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.b="SCnBuQtr"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3z2fHV2Cd8z9s8J for ; Fri, 22 Dec 2017 04:47:48 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references; q=dns; s= default; b=OSd7IWgtu2h5xwwlDK1y61I3hpaaVSS25mkJ1vhmu3/B54atQoUyd USp3K3CucaGaglWzfLBv47StHeXTKpQlokMWxLrDz532W/jxA1+FUOSre538pRTi BMI1MhjgDBdDuv1Xk3ob04ULFAj7IHE+Hv9C8e5HtXZybYd2kFXVEY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references; s= default; bh=uQKpRr4TBg+dp5Fa/QeanUADDTk=; b=SCnBuQtrVIVZ9J2OOqFs P7aVw2YeueER/I9d3d6zqeMUKVgE9W36p7OASbiMqUEYBdF8ghHvN6dMzBlcPUlb bMTMJebuoCx9PxxJrpMC75aMXlkVKeic+JrxRzppd2xPids9O8eK90QgXkAiRX7K 0K03E/Y34m9g903SoafoM5E= Received: (qmail 3536 invoked by alias); 21 Dec 2017 17:47:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 3221 invoked by uid 89); 21 Dec 2017 17:47:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Dec 2017 17:47:37 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 97884267F5; Thu, 21 Dec 2017 17:47:36 +0000 (UTC) Received: from c64.redhat.com (ovpn-112-35.phx2.redhat.com [10.3.112.35]) by smtp.corp.redhat.com (Postfix) with ESMTP id 081355D962; Thu, 21 Dec 2017 17:47:34 +0000 (UTC) From: David Malcolm To: Jason Merrill Cc: Nathan Sidwell , Jakub Jelinek , Richard Biener , gcc-patches List , David Malcolm Subject: [v3 of PATCH 08/14] cp/tree.c: strip location wrappers in lvalue_kind Date: Thu, 21 Dec 2017 12:50:59 -0500 Message-Id: <1513878659-57155-1-git-send-email-dmalcolm@redhat.com> In-Reply-To: References: X-IsSubscribed: yes On Wed, 2017-12-20 at 23:56 -0500, Jason Merrill wrote: > On Wed, Dec 20, 2017 at 5:14 PM, David Malcolm > wrote: > > On Mon, 2017-12-11 at 18:39 -0500, Jason Merrill wrote: > > > On 11/10/2017 04:45 PM, David Malcolm wrote: > > > > Without this, then lvalue_p returns false for decls, and hence > > > > e.g. uses of them for references fail. > > > > > > > > Stripping location wrappers in lvalue_kind restores the correct > > > > behavior of lvalue_p etc. > > > > > > > > gcc/cp/ChangeLog: > > > > * tree.c (lvalue_kind): Strip any location wrapper. > > > > > > Rather, lvalue_kind should learn to handle VIEW_CONVERT_EXPR. > > This patch does so, using: > > > > case NON_LVALUE_EXPR: > > case VIEW_CONVERT_EXPR: > > if (location_wrapper_p (ref)) > > return lvalue_kind (TREE_OPERAND (ref, 0)); > > > > As well as the VIEW_CONVERT_EXPR, lvalue_kind needs to handle > > NON_LVALUE_EXPR, otherwise a location-wrapped string literal > > hits this clause in the "default" case: > > > > if (CLASS_TYPE_P (TREE_TYPE (ref)) > > || TREE_CODE (TREE_TYPE (ref)) == ARRAY_TYPE) > > return clk_class; > > > > when it should have hit this one (after removing the > > location wrapper): > > > > case STRING_CST: > > case COMPOUND_LITERAL_EXPR: > > return clk_ordinary; > > Ah, the issue is that string literals should use VIEW_CONVERT_EXPR > rather than NON_LVALUE_EXPR, since they are lvalues. With that > change, we shouldn't need to handle NON_LVALUE_EXPR specifically. > > Jason Thanks. Here's an updated version of the patch which tweaks maybe_wrap_with_location (from patch 02/14 [1]) for STRING_CST to do so, also adding some more test coverage to selftest::test_location_wrappers for the STRING_CST case. With that, it only adds the VIEW_CONVERT_EXPR case to lvalue_kind. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu, as part of the kit. Manually tested with "make s-selftest-c++" (since we don't run the selftests for cc1plus by default). OK for trunk once the rest of the kit is approved? [1] https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02611.html gcc/cp/ChangeLog: * cp-lang.c (selftest::run_cp_tests): Call selftest::cp_tree_c_tests. * cp-tree.h (selftest::run_cp_tests): Move decl to bottom of file. (selftest::cp_tree_c_tests): New decl. * tree.c: Include "selftest.h". (lvalue_kind): Handle VIEW_CONVERT_EXPR location wrapper nodes. (selftest::test_lvalue_kind): New function. (selftest::cp_tree_c_tests): New function. gcc/ChangeLog: * tree.c (maybe_wrap_with_location): Use VIEW_CONVERT_EXPR for STRING_CST. (selftest::test_location_wrappers): Add coverage for STRING_CST. * tree.h (location_wrapper_p): Update comment to reflect change to STRING_CST. --- gcc/cp/cp-lang.c | 1 + gcc/cp/cp-tree.h | 16 ++++++++------ gcc/cp/tree.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ gcc/tree.c | 19 +++++++++++++--- gcc/tree.h | 4 ++-- 5 files changed, 95 insertions(+), 11 deletions(-) diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c index 805319a..9bb4ce7 100644 --- a/gcc/cp/cp-lang.c +++ b/gcc/cp/cp-lang.c @@ -247,6 +247,7 @@ run_cp_tests (void) c_family_tests (); /* Additional C++-specific tests. */ + cp_tree_c_tests (); } } // namespace selftest diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 9879e16..bb405f2 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7415,12 +7415,6 @@ extern tree cp_ubsan_maybe_instrument_downcast (location_t, tree, tree, tree); extern tree cp_ubsan_maybe_instrument_cast_to_vbase (location_t, tree, tree); extern void cp_ubsan_maybe_initialize_vtbl_ptrs (tree); -#if CHECKING_P -namespace selftest { - extern void run_cp_tests (void); -} // namespace selftest -#endif /* #if CHECKING_P */ - /* Inline bodies. */ inline tree @@ -7451,6 +7445,16 @@ named_decl_hash::equal (const value_type existing, compare_type candidate) return candidate == name; } +#if CHECKING_P +namespace selftest { + extern void run_cp_tests (void); + + /* Declarations for specific families of tests within cp, + by source file, in alphabetical order. */ + extern void cp_tree_c_tests (void); +} // namespace selftest +#endif /* #if CHECKING_P */ + /* -- end of C++ */ #endif /* ! GCC_CP_TREE_H */ diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 0ae2eff..6900bc7 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "flags.h" +#include "selftest.h" static tree bot_manip (tree *, int *, void *); static tree bot_replace (tree *, int *, void *); @@ -240,6 +241,11 @@ lvalue_kind (const_tree ref) case NON_DEPENDENT_EXPR: return lvalue_kind (TREE_OPERAND (ref, 0)); + case VIEW_CONVERT_EXPR: + if (location_wrapper_p (ref)) + return lvalue_kind (TREE_OPERAND (ref, 0)); + /* Fallthrough. */ + default: if (!TREE_TYPE (ref)) return clk_none; @@ -5339,4 +5345,64 @@ lang_check_failed (const char* file, int line, const char* function) } #endif /* ENABLE_TREE_CHECKING */ +#if CHECKING_P + +namespace selftest { + +/* Verify that lvalue_kind () works, for various expressions, + and that location wrappers don't affect the results. */ + +static void +test_lvalue_kind () +{ + location_t loc = BUILTINS_LOCATION; + + /* Verify constants and parameters, without and with + location wrappers. */ + tree int_cst = build_int_cst (integer_type_node, 42); + ASSERT_EQ (clk_none, lvalue_kind (integer_zero_node)); + + tree wrapped_int_cst = maybe_wrap_with_location (int_cst, loc); + ASSERT_TRUE (location_wrapper_p (wrapped_int_cst)); + ASSERT_EQ (clk_none, lvalue_kind (integer_zero_node)); + + tree string_lit = build_string (4, "foo"); + TREE_TYPE (string_lit) = char_array_type_node; + string_lit = fix_string_type (string_lit); + ASSERT_EQ (clk_ordinary, lvalue_kind (string_lit)); + + tree wrapped_string_lit = maybe_wrap_with_location (string_lit, loc); + ASSERT_TRUE (location_wrapper_p (wrapped_string_lit)); + ASSERT_EQ (clk_ordinary, lvalue_kind (wrapped_string_lit)); + + tree parm = build_decl (UNKNOWN_LOCATION, PARM_DECL, + get_identifier ("some_parm"), + integer_type_node); + ASSERT_EQ (clk_ordinary, lvalue_kind (parm)); + + tree wrapped_parm = maybe_wrap_with_location (parm, loc); + ASSERT_TRUE (location_wrapper_p (wrapped_parm)); + ASSERT_EQ (clk_ordinary, lvalue_kind (parm)); + + /* Verify that lvalue_kind of std::move on a parm isn't + affected by location wrappers. */ + tree rvalue_ref_of_parm = move (parm); + ASSERT_EQ (clk_rvalueref, lvalue_kind (rvalue_ref_of_parm)); + tree rvalue_ref_of_wrapped_parm = move (wrapped_parm); + ASSERT_EQ (clk_rvalueref, lvalue_kind (rvalue_ref_of_wrapped_parm)); +} + +/* Run all of the selftests within this file. */ + +void +cp_tree_c_tests () +{ + test_lvalue_kind (); +} + +} // namespace selftest + +#endif /* #if CHECKING_P */ + + #include "gt-cp-tree.h" diff --git a/gcc/tree.c b/gcc/tree.c index b72f963..517f4a4 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -13811,8 +13811,8 @@ set_source_range (tree expr, source_range src_range) /* Return EXPR, potentially wrapped with a node expression LOC, if !CAN_HAVE_LOCATION_P (expr). - NON_LVALUE_EXPR is used for wrapping constants. - VIEW_CONVERT_EXPR is used for wrapping non-constants. + NON_LVALUE_EXPR is used for wrapping constants, apart from STRING_CST. + VIEW_CONVERT_EXPR is used for wrapping non-constants and STRING_CST. Wrapper nodes can be identified using location_wrapper_p. */ @@ -13834,7 +13834,7 @@ maybe_wrap_with_location (tree expr, location_t loc) if (EXCEPTIONAL_CLASS_P (expr)) return expr; - if (CONSTANT_CLASS_P (expr)) + if (CONSTANT_CLASS_P (expr) && TREE_CODE (expr) != STRING_CST) return build1_loc (loc, NON_LVALUE_EXPR, TREE_TYPE (expr), expr); else return build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (expr), expr); @@ -14238,6 +14238,18 @@ test_location_wrappers () ASSERT_EQ (loc, EXPR_LOCATION (wrapped_int_cst)); ASSERT_EQ (int_cst, tree_strip_any_location_wrapper (wrapped_int_cst)); + /* Wrapping a STRING_CST. */ + tree string_cst = build_string (4, "foo"); + ASSERT_FALSE (CAN_HAVE_LOCATION_P (string_cst)); + ASSERT_FALSE (location_wrapper_p (string_cst)); + + tree wrapped_string_cst = maybe_wrap_with_location (string_cst, loc); + ASSERT_TRUE (location_wrapper_p (wrapped_string_cst)); + ASSERT_EQ (VIEW_CONVERT_EXPR, TREE_CODE (wrapped_string_cst)); + ASSERT_EQ (loc, EXPR_LOCATION (wrapped_string_cst)); + ASSERT_EQ (string_cst, tree_strip_any_location_wrapper (wrapped_string_cst)); + + /* Wrapping a variable. */ tree int_var = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier ("some_int_var"), @@ -14258,6 +14270,7 @@ test_location_wrappers () /* Verify that STRIP_NOPS removes wrappers. */ check_strip_nops (wrapped_int_cst, int_cst); + check_strip_nops (wrapped_string_cst, string_cst); check_strip_nops (wrapped_int_var, int_var); /* operand_equal_p should "see through" any location wrapper... */ diff --git a/gcc/tree.h b/gcc/tree.h index de085ca..7f4ea5e 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -3679,8 +3679,8 @@ location_wrapper_p (const_tree exp) /* A wrapper node has code NON_LVALUE_EXPR or VIEW_CONVERT_EXPR, and the same type as its operand. - NON_LVALUE_EXPR is used for wrapping constants. - VIEW_CONVERT_EXPR is used for wrapping non-constants. + NON_LVALUE_EXPR is used for wrapping constants, apart from STRING_CST. + VIEW_CONVERT_EXPR is used for wrapping non-constants and STRING_CST. A subtlety is that we may have to test whether we have the correct TREE_CODE for the wrapped TREE_CODE. Otherwise, e.g. the C++ expression: