From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30341 invoked by alias); 8 Apr 2014 12:49:39 -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 30329 invoked by uid 89); 8 Apr 2014 12:49:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 08 Apr 2014 12:49:36 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 704781160D5; Tue, 8 Apr 2014 08:49:34 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id CGcpBsRs-3GD; Tue, 8 Apr 2014 08:49:34 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 468721160D2; Tue, 8 Apr 2014 08:49:34 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id E0260E0BF1; Tue, 8 Apr 2014 05:49:35 -0700 (PDT) Date: Tue, 08 Apr 2014 12:49:00 -0000 From: Joel Brobecker To: "Agovic, Sanimir" Cc: "gdb-patches@sourceware.org" Subject: Re: [PATCH v5 03/15] type: add c99 variable length array support Message-ID: <20140408124935.GG4250@adacore.com> References: <1391704056-25246-1-git-send-email-sanimir.agovic@intel.com> <1391704056-25246-4-git-send-email-sanimir.agovic@intel.com> <20140228162722.GA9582@adacore.com> <0377C58828D86C4588AEEC42FC3B85A71D84DD13@IRSMSX105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0377C58828D86C4588AEEC42FC3B85A71D84DD13@IRSMSX105.ger.corp.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-04/txt/msg00096.txt.bz2 > I addressed all issues (see below) except for these two: > > > > + if (!attr_to_dynamic_prop (attr, die, cu, &high)) > > > { > > > attr = dwarf2_attr (die, DW_AT_count, cu); > > > if (attr) > > > > I know that in the testcase you are trying to support, the bounds > > are necessarily starting at zero and therefore constant/non-dynamic. > > But can you modify the function to also handle the DW_AT_lower_bound > > the same way? Other languages such as Ada will need that also, and > > that seems like the logical time to be doing it. > > > The motivation behind the c99 patch series is to introduce the core concept > of dynamic properties and thus we like to keep it small. Based on that work > the Fortran patch series will fill the gap. OK with me if you prefer to handle this case as a follow up patch. > > > +int > > > +is_dynamic_type (const struct type *type) > > > +{ > > > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY > > > + && TYPE_NFIELDS (type) == 1) > > > + { > > > + const struct type *range_type = TYPE_INDEX_TYPE (type); > > > + > > > + if (!has_static_range (TYPE_RANGE_DATA (range_type))) > > > + return 1; > > > + } > > > + > > > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY > > > + || TYPE_CODE (type) == TYPE_CODE_PTR > > > + || TYPE_CODE (type) == TYPE_CODE_REF > > > + || TYPE_CODE (type) == TYPE_CODE_TYPEDEF) > > > + return is_dynamic_type (check_typedef (TYPE_TARGET_TYPE (type))); > > As discussed on IRC, I think that TYPE_NFIELDS should always > > be 1 for TYPE_CODE_ARRAY. So let's transform that into an assert, > > if you don't mind, and see what happens? > > Can you explain why you included TYPE_CODE_PTR in the list? > > > > This is not clear to me based on the function's description. > > The risk I see with this is trying to print the value of a pointer > > to a dynamic array - the printing really only needs to display > > the address, and not resolve the underlying array. Also, from > > a logical standpoint, pointers are not dynamic types. > > > I may remove the pointer chasing from the patch series, but this means > we won't support the following feature: > > 1| int foo(int vla[n][m]) > > (gdb) ptype vla > type = int (*)[] > (gdb) print vla[0] > Cannot perform pointer math on incomplete types, try casting to a known type, or void *. > > So we included TYPE_CODE_PTR to support such use case. OK. Do we have a test for this in the patches you propose? I think it's important to have one. That way, if anyone like me just tries to remove it, it'll show up in the testsuite. > > Can you explain in this comment what the referenced type is? > > If it helps, giving an example could be used. > > > Done, see my inlined my changes: Thank you. Small comment (see below): > -- >8 -- > diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h > index 644c546..bcc02694 100644 > --- a/gdb/dwarf2loc.h > +++ b/gdb/dwarf2loc.h > @@ -144,13 +144,14 @@ struct dwarf2_loclist_baton > }; > > /* A dynamic property is either expressed as a single location expression > - or a location list. If the property is a reference store its targeted > - type in TYPE. */ > + or a location list. If the property is an indirection, pointing to ^^ missing second space after period. Ok - I think we're ready for v6 of the patch series. Let's hope that'll be it! :) Thanks, -- Joel