From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26384 invoked by alias); 2 Aug 2005 03:10:56 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 26377 invoked by uid 22791); 2 Aug 2005 03:10:53 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Tue, 02 Aug 2005 03:10:53 +0000 Received: from drow by nevyn.them.org with local (Exim 4.52) id 1DznB5-0004GQ-Ca; Mon, 01 Aug 2005 23:10:51 -0400 Date: Tue, 02 Aug 2005 03:10:00 -0000 From: Daniel Jacobowitz To: Wu Zhou Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] New testcase to evaluate Fortran substring expression Message-ID: <20050802031051.GC29761@nevyn.them.org> Mail-Followup-To: Wu Zhou , gdb-patches@sources.redhat.com References: <20050703185733.GI13811@nevyn.them.org> <20050714234612.GA21620@nevyn.them.org> <20050801021253.GH30901@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.8i X-SW-Source: 2005-08/txt/msg00037.txt.bz2 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