Re: [gfortran, PR33254] Add -fbounds-checking for character array constructors

[Tobias Schlüter <tobias dot schlueter at physik dot uni-muenchen dot de>]

Tobias Burnus wrote:
> Tobias Schlüter wrote:
> > the patch is straightforward, and does exactly what is advertised in
> > the subject line.  Unfortunately, I discovered PR33727 while preparing
> > this patch.  One PR down, still the same number to go.
> Reviewing had been faster, if you used ADDPATCH ...

It's not required by our policies :-)  That said, I do try to always add 
the line.

> (And, since I started ranting, it would be great if FX were using the PR
> number in the subject or at least in the message body; currently I have
> to save his patches (MIME application/octet-stream) to find out whether
> there is a PR.)

Same her, I usually go to the mailing list archives to look at them. 
Same with Paul's patches.  My mailer (Thunderbird) can be forced to give 
the right MIME-type by adding the .txt extension after .diff, I would 
welcome it if both of you did the same or found another solution for 
this problem.

> > Built and tested on i386-darwin.  Ok?
> I think the patch is ok, however: The bounds check is done
> unconditionally whereas the subject implies that it is only done with
> -fbounds-check.
>
> (I'm slightly inclined to prefer to have the check with -fbounds-check
> only, but I am also satisfied with an unconditional error.)
>
> Other compilers:
> a) NAG f95 and g95 always give a run-time error
> b) ifort, sunf95, openf95 give no error (not even with bounds checking
> enabled)
>
> Current gfortran does (b), with the patch it does (a)

From trans-array.c:

static tree
gfc_trans_array_bound_check (gfc_se * se, tree descriptor, tree index, 
int n,
			     locus * where, bool check_upper)
{
  tree fault;
  tree tmp;
  char *msg;
  const char * name = NULL;

  if (!flag_bounds_check)
    return index;

So I do think the patch adds the check only with bounds-checking 
enabled.  I'm rebuilding to check this.

As for whether the check is desirable at runtime, even without 
bounds-checking, I've no real opinion.  CHARACTER array constructors are 
expensive, so there's not much additional cost involved in always 
checking them, and I could modify my patch for this if desired.

Cheers,
- Tobi