Re: [RFA] New testcase to evaluate Fortran substring expression

[Daniel Jacobowitz <drow@false.org>]

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