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

[Andre Vehreschild]

Hi Janus, hi Paul, hi Tobias,

Janus: During code review, I found that I had the code in
gfc_get_len_component() duplicated. So I now reintroduced and documented the
routine making is more commonly usable and added more documentation. The call
sites are now simplify.c (gfc_simplify_len) and trans-expr.c
(gfc_trans_pointer_assignment). Attached is the reworked version of the patch.

Paul, Tobias: Can one of you have a look at line 253 of the patch? I need some
expertise on the bind_c behavior. My patch needs the check for is_bind_c added
in trans_expr.c (gfc_conv_expr) to prevent mistyping an associated variable
in a select type() during the conv. Background: This code fragment taken from
the testcase in the patch:

MODULE m
contains
  subroutine bar (arg, res)
    class(*) :: arg
    character(100) :: res
    select type (w => arg)
      type is (character(*))
        write (res, '(I2)') len(w)
    end select
  end subroutine
END MODULE

has the conditions required for line trans-expr.c:6630 of gfc_conv_expr when
the associate variable w is converted. This transforms the type of the
associate variable to something unexpected in the further processing leading to
some issues during fortraning. Janus told me, that the f90_type has been abused
for some other things (unlimited polymorphic treatment). Although I believe
that reading the comments above the if in question, the check I had to enhance
is treating bind_c stuff (see the threads content for more). I would feel safer
when one of you gfortran gurus can have a look and given an opinion, whether
the change is problematic. I couldn't figure why w is resolved to meet the
criteria (any ideas). Btw, all regtest are ok reporting no issues at all.

Bootstraps and regtests ok on x86_64-linux-gnu

Regards,
	Andre


On Sat, 3 Jan 2015 16:45:07 +0100
Janus Weil <janus@gcc.gnu.org> wrote:

> Hi Andre,
> 
> >> >> 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.
> >>
> >> I don't understand how f90_type can be BT_VOID without being in a
> >> BIND_C context, but I'm not really a ISO_C_BINDING expert. Which test
> >> case is the one that triggered this?
> >
> > This case is triggered by the test-case in the patch, where in the select
> > type (w => arg) in module m routine bar the w meets the criteria to make the
> > condition become true. The type of w is then "fixed" and gfortran would
> > terminate, because the type of w would be set be and BT_INTEGER. I tried to
> > backtrace where this is coming from, but to no success. In the resolve () of
> > the select type it looks all quite ok, but in the trans stage the criteria
> > are met. Most intriguing to me is, that in the condition we are talking
> > about the type of w and f90_type of the derived class' ts
> > (expr->ts.u.derived->ts.f90_type) of w is examined. But
> > expr->ts.u.derived->ts does not describe the type of w, but of the class w
> > is associate with __STAR...
> >
> > So I am not quite sure how to fix this, if this really needs fixing. When I
> > understand you right, then f90_type should only be set in a bind_c context,
> > so adding that check wouldn't hurt, right?
> 
> Yes, in principle adding the check for attr.bind_c looks ok to me
> (alternatively one could also check for attr.unlimited_polymorphic). I
> think originally BT_VOID was indeed only used in a bind_c context, but
> recently it has also been 'hijacked' for unlimited polymorphism, e.g.
> for the STAR symbol and some of the components of the intrinsic vtabs.
> 
> What I don't really understand is why these problems are triggered by
> your patch now and have not crept up earlier in other use-cases of
> CLASS(*).
> 
> 
> >> >> 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?
> >>
> >> I leave that up to you. In principle I'm fine with keeping it as it
> >> is. The only problem I see is that the function name sounds rather
> >> general, but it apparently expects the expression to be an ASSOCIATE
> >> symbol.
> >
> > I am nearly finished with the patch on allocatable scalar components and I
> > don't need the code there. Therefore I have inlined the routine.
> 
> Ok, good. Could you please post an updated patch?
> 
> 
> > So, what do we do about the bind_c issue above? Is some bind_c guru
> > available to have a look at this? It would be very much appreciated.
> 
> From my non-guru POV, it can stay as is.
> 
> It would be helpful if someone like Paul or Tobias could have a look
> at the patch before it goes to trunk. I think it's pretty close to
> being ready for prime-time. Thanks for your work!
> 
> Cheers,
> Janus


-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr60255_7.clog
Type: application/octet-stream
Size: 1231 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20150104/8dcf7c76/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr60255_7.patch
Type: text/x-patch
Size: 21241 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20150104/8dcf7c76/attachment.bin>