From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76616 invoked by alias); 19 Apr 2019 21:00:43 -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 76552 invoked by uid 89); 19 Apr 2019 21:00:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=TYPE_CODE_CHAR, H*i:sk:7134c1a, type_code_int, Empty X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Apr 2019 21:00:37 +0000 Received: by mail-wr1-f66.google.com with SMTP id c12so2028255wrt.8 for ; Fri, 19 Apr 2019 14:00:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Mt+1WwH0kS3xeLPIZ4Nes98k1Lr5ib9rpgGhmt5d5pU=; b=Li1AlXXaqbqZq32xdMoE+Tylk2apJNSbDU4EZuh7mL3f/ieGS1EHgGU6PxcKo94nau Fs4+eYJFOC8xRCC3HWMm4sj/qNTyPPclKyQpIkbYjnGlT2yVpdP+S4rLAg0E945fRloJ hSzkWWWOhv1E59DNSKD0lVdXenJa8+9gSnbVXl1POUPleEpVuNlN3rWcDaks/d80KOdB pk9qqPrm1V+vKzon3m99e0qKNq+E+lb4SixzXG/frpWldyHWo/IxvsuvnxlgCtTaGQct Cy9p45irglEGBcD9UiDiSjN6KvuJFlUqo/xqnEhFcBaLWLDXHacX88uKTH+n76SivLRd fajA== Return-Path: Received: from localhost (host109-148-134-137.range109-148.btcentralplus.com. [109.148.134.137]) by smtp.gmail.com with ESMTPSA id t76sm7967300wmt.8.2019.04.19.14.00.33 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 19 Apr 2019 14:00:34 -0700 (PDT) Date: Fri, 19 Apr 2019 21:00:00 -0000 From: Andrew Burgess To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCHv2 4/5] gdb: Introduce new language field la_is_string_type_p Message-ID: <20190419210033.GV2737@embecosm.com> References: <7134c1a3-8cd3-bcb2-c1b3-444f1c879049@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7134c1a3-8cd3-bcb2-c1b3-444f1c879049@redhat.com> X-Fortune: This fortune intentionally not included. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-04/txt/msg00391.txt.bz2 * Pedro Alves [2019-04-18 18:07:55 +0100]: > On 4/17/19 12:06 AM, Andrew Burgess wrote: > > This commit is preparation work for the next commit, and by itself > > makes no user visible change to GDB. I've split this work into a > > separate commit in order to make code review easier. > > > > This commit adds a new field 'la_is_string_type_p' to the language > > struct, this predicate will return true if a type is a string type for > > the given language. > > > > Some languages already have a "is this a string" predicate that I was > > able to reuse, while for other languages I've had to add a new > > predicate. In this case I took inspiration from the value printing > > code for that language - what different conditions would result in > > printing something as a string. > > > > A default "is this a string" method has also been added that looks for > > TYPE_CODE_STRING, this is the fallback I've used for a couple of > > languages. > > > > In this commit I add the new field and initialise it for each > > language, however at this stage the new field is never used. > > > > Thanks. This nicely addresses one of my previous review comments. > > I can't speak to whether the implementation for the different languages > are correct. E.g., I'm curious from where you extracted the M2 and > the Rust bits. Didn't seem like it was a refactoring job? It was mostly me looking for calls to LA_PRINT_STRING in each languages value printing code then combining all of the conditions required to reach these calls. Could we refactor to make use of these new is_string functions within the value printing code? Sure, but I don't think it would be pretty. Most languages base their value printing around a switch statement, and also have additional format related conditions mixed in with the logic. Ideally, at least for this commit I'd prefer to no mess with the value printing code more than I need too... Thanks, Andrew > > Formatting comments below. > > > +/* Return true if TYPE is a string. */ > > +static bool > > Empty line after comment. > > > +static bool > > +m2_is_string_type_p (struct type *type) > > +{ > > + type = check_typedef (type); > > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY > > + && TYPE_LENGTH (type) > 0 > > + && TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0) > > + { > > + struct type *elttype = check_typedef (TYPE_TARGET_TYPE (type)); > > + > > + if (TYPE_LENGTH (elttype) == 1 && > > && goes on the next line. > > > + (TYPE_CODE (elttype) == TYPE_CODE_INT > > + || TYPE_CODE (elttype) == TYPE_CODE_CHAR)) > > + return true; > > + } > > + > > + return false; > > +} > > + > > Thanks, > Pedro Alves