[Patch, Fortran] SELECT TYPE via ASSOCIATE

[Daniel Kraft]

Hi Janus,

thanks for your review / comments!

Janus Weil wrote:
> Hi Daniel,
> 
>> the attached patch fixes ASSOCIATE for polymorphic values and switches the
>> current implementation of SELECT TYPE to using ASSOCIATE internally.
> 
> thanks for working on this. I only had a glance at your patch, but
> what I've seen looks good so far. Just some comments ...
> 
> 
>> As a side-effect, this fixes the "double-free" PRs 44047 and 45384.
> 
> Ok. You told me before that you don't do anything special to prevent
> these double free issues with ASSOCIATE. Can you explain to me how
> your patch actually fixes these? I'd just like to understand it.

Honestly, I don't have a really good explanation -- for one, I do not 
even understand why there were two free's in your old case.  As I saw 
it, what you did was basically to build a POINTER variable and point it 
to the result in the front-end.  Why was it auto-deallocated (being a 
POINTER!)?

But I think the main difference now is that ASSOCIATE is handled in the 
trans-* phase.  Thus I have complete control over the initialization and 
clean-up code (see trans_assoc_var or what it is called in trans-decl.c) 
instead of building something up in the front-end and "hoping" that 
trans-* does what I we want it to do.

> One difference I observed in the dump of PR 45384 (comment #3) is that
> the old version had:
> 
>         struct class$d_base_sparse_mat_a aa;
> 
> while with your patch one gets:
> 
>         struct class$d_base_sparse_mat_a * aa;
> 
> I.e. one now has a pointer to a class container. This is a thing we
> usually don't have in the OOP implementation, not even for
> 
> class(...), pointer :: x
> 
> since the pointer attribute is propagated to the $data component of
> the class container, and the container itself always appears as
> non-pointer. Therefore I think it might be better to stick with the
> old non-pointer declaration. However, the pointer might just be the
> reason that auto-deallocation does not happen. Right?

This is also a consequence of my ASSOCIATE implementation in the 
backend.  I.e., associate-names are not POINTERs assigned to the target, 
but instead they are smoething "special" and trans-* takes care of them 
-- building a real pointer to whatever the object in question is (for 
scalar quantities; for arrays, a new descriptor is built and assigned to).

You surely know gfortran's OOP much better than I do, so I leave it to 
you to decide whether this is ok or not (it did not fail for any of my 
tests so far).  If the latter, we should probably implement something 
similar to arrays and create a new class-container in the backend linked 
to the target (or something like that).

As I wrote above, the big difference is that ASSOCIATE is done directly 
in trans (because I think it could not be done reliably for stuff like 
arrays in the front-end); this somehow fixes the double-free problem. 
I'd like to stick with the current implementation (real pointer to 
class-container rather than the _p struct) if this is possible without 
really breaking anything (because this is the "natural" implementation 
that works without further special cases), but if you have a good reason 
why this is not correct for CLASS, I'll add special handling to the 
ASSOCIATE code.

>> Regtested on GNU/Linux-x86-32.  The only failure was bessel_7.f90, which
>> goes away when I increase the tolerance according to
>> http://gcc.gnu.org/ml/fortran/2010-08/msg00308.html.  Ok for trunk?
> 
> Pretty much ok from my side, except for the points mentioned above.

So I'm waiting on your impression of my answers above ;)

> Btw, for select_type_13.f03 you might wanna give credit to Salvatore
> instead of me, since it's basically his test case (and he puts a lot
> of effort into testing gfortran, so he very much deserved to be
> mentioned).

Ok, I'll switch this ;)  We could of course also use his original 
test-case from the PR, but I think the reduced one is better suited for 
the test-suite.

Yours,
Daniel

-- 
http://www.pro-vegan.info/
--
Done:  Arc-Bar-Cav-Kni-Ran-Rog-Sam-Tou-Val-Wiz
To go: Hea-Mon-Pri