From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 73772 invoked by alias); 15 Sep 2017 20:22: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 73742 invoked by uid 89); 15 Sep 2017 20:22:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=3703 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; Fri, 15 Sep 2017 20:22: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 v8FKM9gs004698 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 15 Sep 2017 16:22:14 -0400 Received: by simark.ca (Postfix, from userid 112) id 49B9C1ECF0; Fri, 15 Sep 2017 16:22:09 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 8ACFA1EA9D; Fri, 15 Sep 2017 16:21:57 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 15 Sep 2017 20:22:00 -0000 From: Simon Marchi To: Tim Wiederhake Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/5] Fortran: Move calc_f77_array_dims. In-Reply-To: <1505134663-29374-2-git-send-email-tim.wiederhake@intel.com> References: <1505134663-29374-1-git-send-email-tim.wiederhake@intel.com> <1505134663-29374-2-git-send-email-tim.wiederhake@intel.com> 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 Fri, 15 Sep 2017 20:22:09 +0000 X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00424.txt.bz2 Hi Tim, The patch is ok, with some nits I noted below. On 2017-09-11 14:57, Tim Wiederhake wrote: > 2017-09-11 Tim Wiederhake > > gdb/ChangeLog: > * eval.c (evaluate_subexp_standard): Use new function name. > (calc_f77_array_dims): Move ... > * f-lang.c (f77_get_array_dims): ... here. Constify argument. Make > NULL check explicit. > * f-lang.h (calc_f77_arra_dims): Rename to... > (f77_get_array_dims): ... this. Add comment. > * f-valprint.c (f77_print_array): Use new function name. > > --- > gdb/eval.c | 21 +-------------------- > gdb/f-lang.c | 16 ++++++++++++++++ > gdb/f-lang.h | 4 +++- > gdb/f-valprint.c | 2 +- > 4 files changed, 21 insertions(+), 22 deletions(-) > > diff --git a/gdb/eval.c b/gdb/eval.c > index 24f32f8..7a808a0 100644 > --- a/gdb/eval.c > +++ b/gdb/eval.c > @@ -2336,7 +2336,7 @@ evaluate_subexp_standard (struct type > *expect_type, > if (nargs > MAX_FORTRAN_DIMS) > error (_("Too many subscripts for F77 (%d Max)"), > MAX_FORTRAN_DIMS); > > - ndimensions = calc_f77_array_dims (type); > + ndimensions = f77_get_array_dims (type); > > if (nargs != ndimensions) > error (_("Wrong number of subscripts")); > @@ -3266,22 +3266,3 @@ parse_and_eval_type (char *p, int length) > error (_("Internal error in eval_type.")); > return expr->elts[1].type; > } > - > -int > -calc_f77_array_dims (struct type *array_type) > -{ > - int ndimen = 1; > - struct type *tmp_type; > - > - if ((TYPE_CODE (array_type) != TYPE_CODE_ARRAY)) > - error (_("Can't get dimensions for a non-array type")); > - > - tmp_type = array_type; > - > - while ((tmp_type = TYPE_TARGET_TYPE (tmp_type))) > - { > - if (TYPE_CODE (tmp_type) == TYPE_CODE_ARRAY) > - ++ndimen; > - } > - return ndimen; > -} > diff --git a/gdb/f-lang.c b/gdb/f-lang.c > index 903cfd1..77b759b 100644 > --- a/gdb/f-lang.c > +++ b/gdb/f-lang.c > @@ -370,3 +370,19 @@ _initialize_f_language (void) > { > f_type_data = gdbarch_data_register_post_init (build_fortran_types); > } > + > +/* See f-lang.h. */ > + > +int > +f77_get_array_dims (const struct type *array_type) > +{ > + if ((TYPE_CODE (array_type) != TYPE_CODE_ARRAY)) > + error (_("Can't get dimensions for a non-array type")); > + > + int ndimen = 0; > + for (; array_type != NULL; array_type = TYPE_TARGET_TYPE > (array_type)) > + if (TYPE_CODE (array_type) == TYPE_CODE_ARRAY) > + ndimen += 1; ndimen++; ? Just a question (just showing off my ignorance of fortran): don't you want to stop looping when you reach a type that is a non-array? I don't know which types in fortran can have a target, but is it possible to have something like this? array type -> array type -> other type -> array type The code above would give a dimension of 3, is it what we want? I would have thought that the "top-level" array has a dimension of 2, but its elements is of type "other type". > + > + return ndimen; > +} > diff --git a/gdb/f-lang.h b/gdb/f-lang.h > index 5633b41..cfe667b 100644 > --- a/gdb/f-lang.h > +++ b/gdb/f-lang.h > @@ -55,7 +55,9 @@ extern int f77_get_lowerbound (struct type *); > > extern void f77_get_dynamic_array_length (struct type *); > > -extern int calc_f77_array_dims (struct type *); > +/* Calculate the number of dimensions of an array. Expects ARRAY_TYPE > to be > + * the type of an array. */ Nit: remove the star at the beginning of the second line. I would suggest this wording: Calculate the number of dimensions of an array of type ARRAY_TYPE. > +extern int f77_get_array_dims (const struct type *array_type); > > > /* Fortran (F77) types */ > diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c > index 8fc894a..59d1a2f 100644 > --- a/gdb/f-valprint.c > +++ b/gdb/f-valprint.c > @@ -180,7 +180,7 @@ f77_print_array (struct type *type, const gdb_byte > *valaddr, > int ndimensions; > int elts = 0; > > - ndimensions = calc_f77_array_dims (type); > + ndimensions = f77_get_array_dims (type); > > if (ndimensions > MAX_FORTRAN_DIMS || ndimensions < 0) > error (_("\ Thanks, Simon