Dear Tobias,
Many thanks for the review and your replies to redux(1)/(2).
I will take account of your comments and commit tomorrow night.
I have a fix for the allocation of class temporaries, which cures the
problem with index vectors.... It will see the light of day on Monday.
Happy New Year to one and all.
Paul
On Sat, Dec 31, 2011 at 12:14 AM, Tobias Burnus <bur...@net-b.de> wrote:
> Dear Paul,
>
> Paul Richard Thomas wrote:
>>
>> Bootstrapped and regtested on i686/Ubuntu 11.1 - OK for trunk?
>>
>> 2011-12-30 Paul Thomas<pa...@gcc.gnu.org>
>>
>> * trans-array.c (gfc_array_allocate): Null allocated memory of
>> newly allocted class arrays.
>
>
> PR fortran/51529
>
>
>> PR fortran/46328
>> * interface.c(build_compcall_for_operator): Add a type to the
>> expression.
>
>
> You might want to quote additionally PR fortran/51052 and PR fortran/51052.
>
>> *** gcc/fortran/interface.c (revision 182566)
>> --- gcc/fortran/interface.c (working copy)
>> ***************
>> *** 3256,3261 ****
>> --- 3256,3269 ----
>
>
> I would have liked a diff with "-p" flag which shows the function name in
> the diff (for instance "svn diff -x -p" or "svn diff -x '-p -u'").
>
>>
>> + 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. */
>
>
> I don't like the comment; how about something along the following: "For
> class objects we need to nullify the memory in case they have allocatable
> components; the reason is that _copy, which is used for initialization,
> first frees the destination."
>
>
>> + gfc_trans_class_init_assign (gfc_code *code)
>> + {
>> + stmtblock_t block;
>> + tree tmp;
>> + gfc_se dst,src,memsz;
>
>
> Space after each comma.
>
>> + gfc_expr *lhs,*rhs,*sz;
>
>
> Ditto.
>
>
>> ***************
>> *** 3301,3306 ****
>> --- 3502,3514 ----
>> {
>> gfc_conv_expr_reference (&parmse, e);
>>
>> + /* Catch base objects that are not variables. */
>> + if (e->ts.type == BT_CLASS
>> + && e->expr_type != EXPR_VARIABLE
>> + && expr&& e == expr->base_expr)
>
>
> The indentation looks wrong.
>
>
>> + for (args= e->value.function.actual; args; args = args->next)
>> + {
>> + if (expr == args->expr)
>> + expr = args->expr;
>> + }
>
>
> Space before the equal sign in "args=". If you want, you can also remove the
> curly braces as they are not required.
>
>> + args= code->expr1->value.function.actual;
>> + for (; args; args = args->next)
>> + {
>> + if (expr == args->expr)
>> + expr = args->expr;
>> + }
>
>
> Ditto.
>
>> get_declared_from_expr (gfc_ref **class_ref, gfc_ref **new_ref,
>> ! gfc_expr *e)
>> --- 5623,5629 ----
>> get_declared_from_expr (gfc_ref **class_ref, gfc_ref **new_ref,
>> ! gfc_expr *e, bool types)
>
>
> I think "types" deserves a comment line in the comment block before the
> function; additionally - and related - I wonder whether the name is well
> chosen. "types" reminds me of "bt" rather than of "bool". In the changelog,
> you use: "Add 'types' argument to switch checking of derived types on or
> off." Thus, "check_types" would be a possible choice.
>
> Otherwise, the patch looks okay to me.
>
> Happy new year to every one!
>
> Tobias
--
The knack of flying is learning how to throw yourself at the ground and miss.
--Hitchhikers Guide to the Galaxy
