Re: [Patch, fortran] - Arrays of classes for fortran

2011-12-11 Thread Paul Richard Thomas
Dear Tobias,

On Sun, Dec 11, 2011 at 7:39 PM, Tobias Burnus  wrote:
> Dear Paul, dear all,
>
> first, thanks again for the patch.

Thank you for the continuous reviewing over the last couple of months
- also to Dominique, Salvatore and Damian; all of whom have kept the
test cases coming in.
>
>
> Paul Richard Thomas wrote:
>>
>> Boostrapped and regtested on x86_64/FC9 - OK for trunk?

Committed as revision 182210 with comments and corrections taken on board.

Cheers

Paul


Re: [Patch, fortran] - Arrays of classes for fortran

2011-12-11 Thread Tobias Burnus

Dear Paul, dear all,

first, thanks again for the patch.

Paul Richard Thomas wrote:

Boostrapped and regtested on x86_64/FC9 - OK for trunk?


I have now re-read the patch and it is OK from my side. It wouldn't harm 
is someone else with experience with CLASS or with the scalarizer could 
also read the patch (before or after committal).


Nits:


* gfortran.dg/auto_dealloc_2.f90: Occurences of __builtin_free
now 2.  * gfortran.dg/auto_dealloc_2.f90: Occurences of __builtin_free
* gfortran.dg/class_19.f03: Occurences of __builtin_free now 8.


Missing line break after "2.".



+   for (ref = expr->ref; ref; ref = ref->next)
+ {
+ if (ref->type == REF_COMPONENT
+   &&  ref->u.c.component->ts.type == BT_CLASS
+   &&  ref->next&&  ref->next->type == REF_COMPONENT
+   &&  strcmp (ref->next->u.c.component->name, "_data") == 0
+   &&  ref->next->next
+   &&  ref->next->next->type == REF_ARRAY
+   &&  ref->next->next->u.ar.type != AR_ELEMENT)
+ {
+   ts =&ref->u.c.component->ts;
+   class_ref = ref;
+   break;
+ } 
+ }


Those lines look wrongly indented.



   /* Generate code to initialize an allocate an array.  Statements are added to


Untouched by your patch, but could you nevertheless fix the sentence by 
changing "an" to "and"?



+ #if 0
+   if (expr->ts.type == BT_CLASS&&  expr3)
+ {
+   tmp = build_int_cst (unsigned_char_type_node, 0);

+   /* With class objects, it is best to play safe and null the
+memory because we cannot know if dynamic types have allocatable
+components or not..
+OOP-TODO: Determine if this is necessary or not.  */


and


+ else if (al->expr->ts.type == BT_CLASS&&  code->expr3)
+   {
+ /* With class objects, it is best to play safe and null the
+memory because we cannot know if dynamic types have allocatable
+components or not.  */
+ tmp = build_call_expr_loc (input_location,
+builtin_decl_explicit 
(BUILT_IN_MEMSET),
+3, se.expr, integer_zero_node,  memsz);
+ gfc_add_expr_to_block (&se.pre, tmp);
+   }


Can the #if 0 ... #endif block be removed? Seemingly, you have not found 
a case where it is not initialized. Additionally, I think the same 
applies to the the second quote: If there is an expr3 (i.e. MOLD= or 
SOURCE=) with a nonpolymorphic or with a polymorphic source-expr, either 
the default initialization or the assignment should take care of the 
allocatable components and similar issues.


If you really prefer to keep it (or them), can you fill a 
missed-optimization PR?



After committal, can you write a quip for 
http://gcc.gnu.org/wiki/GFortran#GCC4.7 ?
(Additionally, http://gcc.gnu.org/wiki/OOP and maybe 
http://gcc.gnu.org/wiki/Fortran2003 could be updated.)


Tobias