[PATCH, fortran, final] PR fortran/60255 Deferred character length

[Andre Vehreschild]

Hi Janus,

thank you for your review. 

> I had a look over the patch, and it looks mostly fine to me. A few remarks:
> 
> 1) There are still two TODO markers in the patch. It might be a good
> idea to take care of them before committing the patch. In particular
> for the first one (adding the initializer in gfc_build_class_symbol)
> it would be good to understand where those problems come from. 

I started with the initializer for the _len component and ran into "Pointer
assignment target is neither TARGET nor POINTER at %L" errors (expr.c:3714). I
tracked this back to the constructor resolve of the class type. Resolving the
constructor somehow concludes, that something needs to be done for the constant
initializer although it is marked artificial. I could not track down the
location that is causing this behavior, or if I need to set a flag in the class
itself to get through with it. I am hoping, that either some fortran guru says
"You just need to do xyz to get it running." or that we conclude to remove the
TODO and the commented instructions (setting a zero value for _len is done where
needed (gfc_conv_structure trans-expr.c:6540)).

> For the
> second one (in gfc_conv_expr), I don't directly see how it's related
> to deferred char-len. Why is this change needed?

That change is needed, because in some rare case where an associated variable
in a "select type ()" is used, then the type and f90_type match the condition
while them not really being in a bind_c context. Therefore I have added
the check for bind_c. Btw, I now have removed the TODO, because that case is
covered by the regression tests. 

> 2) You're making a lot of changes to 'trans_associate_var', but I
> don't see any ASSOCIATE statements covered in your test case. Can you
> add more test cases which cover this code?

Select type (assoc => upoly) uses these where an explicit assoc is supplied.
The many changes are needed to migrate from using _vptr%size to then _len
component. All these changes are covered by existing regression tests starting
from unlimited_polymorphic_N.* to the character_length tests. The remaining open
cases not covered by existing tests are in unlimited_polymorphic_20.f03.

> 3) The function 'gfc_get_len_component' that you're introducing is
> only called in a single place. Do you expect this to be useful in
> other places in the future, or could one remove the function and
> insert the code inline?

In one of the first versions it was uses from two locations. But I had to
remove one call site again. I am currently not sure, if I will be using it in
the patch for allocatable components when deferred char arrays are handled. So
what I do I do now? Inline it and when needed make it explicit again in a
future patch?

> 4) You're adding a prototype for a function
> 'gfc_assign_charlen_to_unlimited_poly' in gfortran.h which never gets
> implemented.

Whoopsie, sorry, removed. 

> 5) The second hunk in find_intrinsic_vtab is a whitespace-only change
> which should not occur at all AFAICS.

Yep, agreed. Misconfigured my IDE. Fixed. Sorry for the noise.


So two open questions remain:
ad 1) How to handle the initializer?
ad 3) What to do with the function?

Can you give me an opinion, then I will change the patch and resubmit.

> In any case, thanks for working on this!

Your welcome and happy new year to you.

Regards,
	Andre

Btw, just cleaning up some oddities in the allocatable component patch for
pr60357/pr61337/pr55901 (and may be covering others). Then that one goes
public, too. (pr60357 is just my working title. I know it is fixed already by
your patch.)

> 
> Cheers,
> Janus
> 
> 
> 
> > On Tue, 30 Dec 2014 16:35:48 +0100
> > Dominique d'Humières <dominiq@lps.ens.fr> wrote:
> >
> >> The new patch fixes the ICEs, but still emit the wrong codes reported in
> >> pr61337.
> >>
> >> Thanks and Happy New Year to all,
> >>
> >> Dominique
> >>
> >> > Le 30 déc. 2014 à 14:39, Andre Vehreschild <vehre@gmx.de> a écrit :
> >> >
> >> > Hi Dominique,
> >> >
> >> > thanks for pointing that out. That was caused by a flaw in the current
> >> > patch. In the attached version this is fixed now.
> >> >
> >> > Bootstraps and regtests ok on x86_64-linux-gnu.
> >> >
> >> > Regards,
> >> >     Andre
> >> >
> >> > On Mon, 29 Dec 2014 16:32:27 +0100
> >> > Dominique d'Humières <dominiq@lps.ens.fr> wrote:
> >> >
> >> >> For the record, compiling the tests in pr61337 with the patch applied on
> >> >> top of r219099 gives ICEs:
> >> >>
> >> >>   use array_list
> >> >> 1
> >> >> internal compiler error: in gfc_advance_chain, at fortran/trans.c:58
> >> >>
> >> >> Since this replaces some wrong-code generation by some ICEs, I don’t
> >> >> think this should delay the fix of pr60255.
> >> >>
> >> >> Cheers,
> >> >>
> >> >> Dominique
> >>
> >
> >
> > --
> > Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
> > Tel.: +49 241 9291018 * Email: vehre@gmx.de


-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de