This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] New testcase to evaluate Fortran substring expression
- From: Daniel Jacobowitz <drow at false dot org>
- To: Wu Zhou <woodzltc at cn dot ibm dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Mon, 1 Aug 2005 23:10:51 -0400
- Subject: Re: [RFA] New testcase to evaluate Fortran substring expression
- References: <Pine.LNX.4.63.0506221044490.11343@wks190384wss.cn.ibm.com> <20050703185733.GI13811@nevyn.them.org> <Pine.LNX.4.63.0507080037480.25810@wks190384wss.cn.ibm.com> <20050714234612.GA21620@nevyn.them.org> <Pine.LNX.4.63.0507141043090.7735@wks190384wss.cn.ibm.com> <Pine.LNX.4.63.0507151821280.21784@wks190384wss.cn.ibm.com> <20050801021253.GH30901@nevyn.them.org> <Pine.LNX.4.63.0507291616040.17779@localhost.localdomain>
Could you try again to fix your clock, please? These dates can't be
right... by the time I sent this you ought to be well on to Monday.
On Fri, Jul 29, 2005 at 04:21:37PM +0800, Wu Zhou wrote:
> On Sun, 31 Jul 2005, Daniel Jacobowitz wrote:
>
> > This is looks very good! Two comments for you:
> >
> > - The magic (and undocumented) constants are not a good idea. Rather
> > than being clever with abs(), how about using an enum saying what
> > sort of range it is?
>
> What about adding the following enumeration definition to f-lang.h and
> keeping the use of abs to get the number of arguments following the F90
> subrange expression:
>
> /* This enumeration type is to identify the sort of F90 subrange expression.
> If only the low bound is by default, set it to -1; if both bounds are by
> default, set it to 0; if only the high bound is by default, set it to 1;
> if no bound is by default, set it to 2. The absolute value of the value
> is also the number of arguments following this expression */
>
> enum f90_range_type
> {
> LOW_BOUND_DEFAULT=-1,
> BOTH_BOUND_DEFAULT,
> HIGH_BOUND_DEFAULT,
> NONE_BOUND_DEFAULT
> };
>
> If use other values, it might looks ugly to me to set the nargs based on
> the type value respectively. What is your point on this?
What's so unpleasant about:
switch (range_type)
{
case LOW_BOUND_DEFAULT:
case HIGH_BOUND_DEFAULT:
num_args = 1;
break;
case BOTH_BOUND_DEFAULT:
num_args = 0;
break;
case NONE_BOUND_DEFAULT:
num_args = 2;
break;
}
The fundamental difference is that this is self-documenting at the
point of use. You don't need to look anything up to understand what it
means.
> > - You have a bunch of lines which are too long in eval.c; could you
> > fix that, please?
>
> Sorry, I am not very sure what is the maximum limit of a line in the sources
> and comments. Maybe it is 72? or 80, or any other number? Maybe I can
> choose to use the smaller one?
As Mark said: I generally try to use 79 columns. I find 72 a little
restrictive.
--
Daniel Jacobowitz
CodeSourcery, LLC