From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51703 invoked by alias); 19 Feb 2016 18:52:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 50051 invoked by uid 89); 19 Feb 2016 18:52:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=value_type, ChangeLogs, Hx-languages-length:2925 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 19 Feb 2016 18:52:43 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 265ECC000715; Fri, 19 Feb 2016 18:52:42 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1JIqfCA012434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 19 Feb 2016 13:52:41 -0500 From: Keith Seitz Subject: Re: [PATCH v2 03/11] [PR gdb/14441] gdb: valops: add ability to return rvalue reference values from value_ref() To: Artemiy Volkov References: <1450661481-31178-1-git-send-email-artemiyv@acm.org> <1453229609-20159-1-git-send-email-artemiyv@acm.org> <1453229609-20159-4-git-send-email-artemiyv@acm.org> Cc: gdb-patches@sourceware.org Message-ID: <56C76479.8020507@redhat.com> Date: Fri, 19 Feb 2016 18:52:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <1453229609-20159-4-git-send-email-artemiyv@acm.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00613.txt.bz2 Just "nits" in this review. On 01/19/2016 10:53 AM, Artemiy Volkov wrote: > 2016-01-19 Artemiy Volkov > > * gdb/ada-lang.c (ada_evaluate_subexp): Adhere to the new > value_ref() interface. > * gdb/c-valprint.c (c_value_print): Likewise. > * gdb/infcall.c (value_arg_coerce): Likewise. > * gdb/python/py-value.c (valpy_reference_value): Likewise. > * gdb/valops.c (value_cast): Likewise. > (value_reinterpret_cast): Likewise. > (value_dynamic_cast): Likewise. > (value_ref): Parameterize by kind of return value reference type. > (typecmp): Likewise. Another place that could be simplified, removing "likewise." > * gdb/value.h: New interface. This isn't a particularly useful comment. ChangeLogs explain what changed: * gdb/value.h (value_ref): Add new parameter "refcode". > diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c > index 62552ec..e210e99 100644 > --- a/gdb/c-valprint.c > +++ b/gdb/c-valprint.c > @@ -602,10 +602,13 @@ c_value_print (struct value *val, struct ui_file *stream, > else if (options->objectprint > && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) > { > - int is_ref = TYPE_CODE (type) == TYPE_CODE_REF; > + int is_ref = TYPE_REFERENCE (type); > + enum type_code refcode = TYPE_CODE_UNDEF; Indentation? > > - if (is_ref) > + if (is_ref) { > val = value_addr (val); > + refcode = TYPE_CODE (type); > + } > > /* Pointer to class, check real type of object. */ > fprintf_filtered (stream, "("); > diff --git a/gdb/valops.c b/gdb/valops.c > index 1aafb5a..cc01689 100644 > --- a/gdb/valops.c > +++ b/gdb/valops.c > @@ -828,7 +829,7 @@ value_dynamic_cast (struct type *type, struct value *arg) > value_address (tem), tem, > rtti_type, &result) == 1) > return value_cast (type, > - is_ref ? value_ref (result) : value_addr (result)); > + is_ref ? value_ref (result, TYPE_CODE (resolved_type)) : value_addr (result)); > > if (TYPE_CODE (resolved_type) == TYPE_CODE_PTR) > return value_zero (type, not_lval); The two changed lines in the above hunk are too long. > @@ -1499,16 +1500,19 @@ value_addr (struct value *arg1) > contents. */ > > struct value * > -value_ref (struct value *arg1) > +value_ref (struct value *arg1, enum type_code refcode) > { > struct value *arg2; > struct type *type = check_typedef (value_type (arg1)); > > - if (TYPE_CODE (type) == TYPE_CODE_REF) > + gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF); > + > + if ((TYPE_CODE (type) == TYPE_CODE_REF || TYPE_CODE (type) == TYPE_CODE_RVALUE_REF) > + && TYPE_CODE (type) == refcode) > return arg1; > The "if ((TYPE_CODE...)" line is too long. > arg2 = value_addr (arg1); > - deprecated_set_value_type (arg2, lookup_lvalue_reference_type (type)); > + deprecated_set_value_type (arg2, lookup_reference_type (type, refcode)); > return arg2; > } > Keith