This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: [PATCH,fortran] Fix PR 31244
- From: Steve Kargl <sgk at troutmask dot apl dot washington dot edu>
- To: FX Coudert <fxcoudert at gmail dot com>
- Cc: fortran at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Fri, 5 Oct 2007 16:41:07 -0700
- Subject: Re: [PATCH,fortran] Fix PR 31244
- References: <20070924221122.GA3766@troutmask.apl.washington.edu> <254EF70E-22A7-470F-A8C3-38CCEC1FEF1B@gmail.com>
On Fri, Oct 05, 2007 at 11:51:06PM +0100, FX Coudert wrote:
> I was clearing my mail backlog, and apparently this patch wasn't
> reviewed, sorry Steve!
No problem. I've been busy.
> >I had an alternative patch that put a sane limit on
> >the repeat count and documented this limit in manual.
> >Personally, I preferred this patch.
>
> Sanity check: you prefer the patch you submitted to the patch adding
> a limit, did I get it right?
I submitted the patch that allows essentially an unlimited
value of the repeat count. It is a GMP integer.
The patch that I did not submit placed a cap of INT_MAX on
the size of repeat count. The error message was changed to
state that a compiler limit was reached/exceeded.
Either patch leads to enormous compile time. If I read the
code correctly, gfortran is populating a singly-linked list
and walking the list takes time.
> >- signed int tmp;
> >- msg = gfc_extract_int (expr, &tmp);
> >+ if (expr->ts.type == BT_INTEGER)
> >+ mpz_set (tail->repeat, expr->value.integer);
> > gfc_free_expr (expr);
>
> I think, to be consistent with what gfc_extract_int() did, we should
> add a check that the expression is an EXPR_CONSTANT of type
> BT_INTEGER, and otherwise call gfc_error(). Maybe this is already
> checked somewhere, in which case I've missed it.
I'll check the code, again. If EXPR_CONSTANT isn't checked currently,
I'll add a some code.
> Also, a formatting question. In the following, is the blank line
> consistent with GCC coding style? (I don't think so, and never put one.)
>
> > static void
> >-resolve_data (gfc_data * d)
> >+resolve_data (gfc_data *d)
> > {
> >+
> > if (resolve_data_variables (d->var) == FAILURE)
> > return;
I could be confusing BSD and GNU styles. If a function has
not local variable declaration, then a blank line is inserted.
func (yada)
{
int x;
if (...)
as oppose to
func (yada)
{
if (...)
I'll check this too.
--
Steve