From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43580 invoked by alias); 14 Sep 2017 13:58:18 -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 43560 invoked by uid 89); 14 Sep 2017 13:58:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 14 Sep 2017 13:58:16 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v8EDw99C009020 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 14 Sep 2017 09:58:14 -0400 Received: by simark.ca (Postfix, from userid 112) id CCC8B1EAA9; Thu, 14 Sep 2017 09:58:09 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id D1F6B1E5B9; Thu, 14 Sep 2017 09:57:48 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 14 Sep 2017 13:58:00 -0000 From: Simon Marchi To: Pedro Alves Cc: Simon Marchi , gdb-patches@sourceware.org, arnez@linux.vnet.ibm.com Subject: Re: [PATCH] Make dwarf_expr_piece::pieces an std::vector In-Reply-To: References: <1505376948-22860-1-git-send-email-simon.marchi@ericsson.com> <83688181-eb4a-b2d6-f614-b011c6220cd3@redhat.com> <20733c26c93b710b8554c149c02d3fb3@polymtl.ca> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 14 Sep 2017 13:58:10 +0000 X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00375.txt.bz2 On 2017-09-14 12:24, Pedro Alves wrote: > On 09/14/2017 10:26 AM, Simon Marchi wrote: >> On 2017-09-14 11:21, Pedro Alves wrote: > >>> I wonder if there's ever a case for 'const T &&' parameters, >>> and if GCC could warn about them. At least in non-template >>> functions. >> > > FYI, I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82213 now. Thanks, I couldn't have stated it as clearly as you did. >> Oops, I started with a const reference, before changing it to rvalue >> reference (and forgot to remove the const). Does the patch look good >> to >> you otherwise? >> > > Sure, with tiny nit below. > >> /* Expand the memory allocated stack to contain at least >> @@ -285,14 +282,9 @@ dwarf_expr_context::stack_empty_p () const >> void >> dwarf_expr_context::add_piece (ULONGEST size, ULONGEST offset) >> { >> - struct dwarf_expr_piece *p; >> + this->pieces.emplace_back (); >> + dwarf_expr_piece *p = &(this->pieces.back ()); >> > > The extra parens look odd to me: > > - dwarf_expr_piece *p = &(this->pieces.back ()); > + dwarf_expr_piece *p = &this->pieces.back (); > > ('p' could be a reference instead, since it's always > non-null, but I guess even better would be to move most > of the code below to a dwarf_expr_piece ctor. But you don't > really have to do that. I definitely understand not wanting > to change too much at a time. And I wouldn't have said a thing > if it weren't for the parens making me pause. :-)) I initially left it as a pointer out of laziness, but I've changed it to a reference now. I am pushing the patch with this and the const rvalue reference fixed. I tried to add some constructors, but I didn't know how to design it nicely to support different types of value locations. Maybe the best would be to add some factory functions that build various kinds of pieces, and make the constructor private. Something like: struct dwarf_expr_piece { static dwarf_expr_piece make_memory_piece (CORE_ADDR addr, bool in_stack_memory) static dwarf_expr_piece make_register_piece (int regno); private: dwarf_expr_piece (); }; I'll try that for a follow-up patch. Simon