From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71462 invoked by alias); 20 Mar 2016 12:10:04 -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 68197 invoked by uid 89); 20 Mar 2016 12:10:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-spam-relays-external:sk:broadba, H*RU:sk:broadba, H*r:sk:broadba, revised X-HELO: mail-lb0-f196.google.com Received: from mail-lb0-f196.google.com (HELO mail-lb0-f196.google.com) (209.85.217.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 20 Mar 2016 12:10:01 +0000 Received: by mail-lb0-f196.google.com with SMTP id bc4so9246308lbc.0 for ; Sun, 20 Mar 2016 05:10:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=tCp7ibI85wuAVs3ZX+8yg4Pg9zZMQzduRaa9YL0NLSA=; b=W2FyREge1TB1zZIrvFy9Qx1w+jP/vX0cVmGo6++AX+O7tpTh7WCHOc6gYdKmhFQ2fL bIlQQZAih9xgO58EBKmUQaRkX4QmqHjNqea/aPfNi8/EDZB68KFkGAfRX+tZEWB8TqKv jfMeESmCbi7E1D2mz/rWCqo3dhdEe8mQpw1pHlIOt5vZTUcf+nZ6JlC+uPrFTgIRWKRQ GbnmalanWiepqeG/qGfA3XPVPi6grKOsc1o9KqMmxZ+jedA2vQBgrZ7vxeaYpQn7orDd dhX/mxBVZR/zdl7V674b55PRmWgZn9Korhp2Kt15BgSYfp+r4dvBXswEGJSSHO89aXk7 TjmA== X-Gm-Message-State: AD7BkJIQMQw3V2hGaKjQiPrFviLMyqCxOZRLP5XPS7sC+vB7I0dYPKd4KJhInhAG/Icgcg== X-Received: by 10.112.199.138 with SMTP id jk10mr9029350lbc.91.1458475798524; Sun, 20 Mar 2016 05:09:58 -0700 (PDT) Received: from gmail.com (broadband-90-154-70-186.nationalcablenetworks.ru. [90.154.70.186]) by smtp.gmail.com with ESMTPSA id r20sm3616837lfd.4.2016.03.20.05.09.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 20 Mar 2016 05:09:57 -0700 (PDT) Date: Sun, 20 Mar 2016 12:10:00 -0000 From: Artemiy Volkov To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 02/11] [PR gdb/14441] gdb: gdbtypes: change {lookup,make}_reference_type() API Message-ID: <20160320120851.GA21558@gmail.com> References: <1453229609-20159-1-git-send-email-artemiyv@acm.org> <1457147955-21871-1-git-send-email-artemiyv@acm.org> <1457147955-21871-3-git-send-email-artemiyv@acm.org> <56E9DBE1.10408@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56E9DBE1.10408@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg00380.txt.bz2 On Wed, Mar 16, 2016 at 03:19:13PM -0700, Keith Seitz wrote: [snip] > > + the kind of reference type to lookup (lvalue or rvalue reference). */ > > > > struct type * > > -make_reference_type (struct type *type, struct type **typeptr) > > +make_reference_type (struct type *type, struct type **typeptr, > > + enum type_code refcode) > > { > > struct type *ntype; /* New type */ > > + struct type **reftype; > > struct type *chain; > > > > - ntype = TYPE_REFERENCE_TYPE (type); > > + gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF); > > + > > + ntype = (refcode == TYPE_CODE_REF ? TYPE_REFERENCE_TYPE (type) > > + : TYPE_RVALUE_REFERENCE_TYPE (type)); > > > > if (ntype) > > { > > @@ -421,7 +427,11 @@ make_reference_type (struct type *type, struct type **typeptr) > > } > > > > TYPE_TARGET_TYPE (ntype) = type; > > - TYPE_REFERENCE_TYPE (type) = ntype; > > + reftype = (refcode == TYPE_CODE_REF ? &TYPE_REFERENCE_TYPE (type) > > + : &TYPE_RVALUE_REFERENCE_TYPE (type)); > > + > > + if(*reftype != NULL) > > + *reftype = ntype; > > > > /* FIXME! Assume the machine has only one representation for > > references, and that it matches the (only) representation for > > I would prefer to do: > > if (refcode == TYPE_CODE_REF) > TYPE_REFERENCE_TYPE (type) = ntype; > else > TYPE_RVALUE_REFERENCE_TYPE (type) = ntype; > > It's cleaner/smaller/easier to read (at least for me). [Otherwise, a > space is missing between "if" and "(".] See next comment, though. My idea was to create a new variable 'reftype' and use it as a pointer to the member of struct main_type that we're dealing with here. Without it, we would have to paste your fragment (more readable, granted) two times rather than just do "*reftype = ntype" two times. I think the latter is better since it's several lines shorter in total and reduces the amount of copy-paste code. > > > > @@ -429,10 +439,9 @@ make_reference_type (struct type *type, struct type **typeptr) > > > > TYPE_LENGTH (ntype) = > > gdbarch_ptr_bit (get_type_arch (type)) / TARGET_CHAR_BIT; > > - TYPE_CODE (ntype) = TYPE_CODE_REF; > > + TYPE_CODE (ntype) = refcode; > > > > - if (!TYPE_REFERENCE_TYPE (type)) /* Remember it, if don't have one. */ > > - TYPE_REFERENCE_TYPE (type) = ntype; > > + *reftype = ntype; > > > > This is slightly different from the original, which only set the new > type if it was unset in the type. Your revised version will > unconditionally do it every time. I have no idea if it makes a > difference or not. Do you? AFAICT *reftype is provably unequal to 0 here, because it was set to ntype by the previous assignment, and ntype was unequal to 0 itself. I went ahead and simplified this a little bit. Is this kind of code change justified here? > > > /* Update the length of all the other variants of this type. */ > > chain = TYPE_CHAIN (ntype); Thanks! Artemiy