This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR 60414: Patch proposal


Hello, thanks for your contribution.

here are some comments about the patch:

Le 21/07/2014 15:03, Andre Vehreschild a écrit :
> diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
> index c33936b..cb01a13 100644
> --- a/gcc/fortran/ChangeLog
> +++ b/gcc/fortran/ChangeLog
Changelogs are preferably provided outside of the patch as they almost
always conflict when applying it.

> @@ -1,3 +1,11 @@
> +2014-07-19  Andre Vehreschild <vehre@gmx.de>
> +
> +	PR fortran/60414
> +	* interface.c (compare_parameter): Fix compile bug: During resolution
> +	of generic an array reference in the actual argument was not
> +	respected. Fixed by checking, if the ref member is non-null.
You don't need to explain why (in the Changelog I mean), only _what_ is
changed should be there.

> Testcase
> +	unlimited_polymorphism_18.f90 add.
No need for that here; it's redundant with the testsuite Changelog.

> +
>  2014-06-15  Tobias Burnus  <burnus@net-b.de>
>
>  	* symbol.c (check_conflict): Add codimension conflict with
> diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
> index b210d18..d84c888 100644
> --- a/gcc/fortran/interface.c
> +++ b/gcc/fortran/interface.c
> @@ -2156,7 +2156,11 @@ compare_parameter (gfc_symbol *formal, gfc_expr
> *actual,
>    if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
>      return 1;
>
> +  /* Only check ranks compatibility, when the actual argument is not a
> +     reference of an array (foo(i)). A reference into an array is assumed
> +     when actual->ref is non null. */
>    if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as
> +        && !actual->ref
>  	&& CLASS_DATA (actual)->as->rank == symbol_rank (formal))
>      return 1;
>
I think you have spotted the right place where the bug happens, but the
patch provided is not completely right for the following reason:

actual->ref points to the beginning of the reference chain, so
for example if actual is the gfc_expr struct for "var%comp%vec",
actual->ref points to the gfc_ref struct for "comp".
Furthermore, you have to be aware that even bare array variables
without sub-reference get an implicit full array reference in the AST, so
if actual is the gfc_expr struct for "array_var", actual->ref is
non-null and points (indirectly) to a gfc_array_ref of type AR_FULL.

So if you want to check for the absence of trailing sub-reference, you
have to walk down to the last reference chain.
But then you have to also support the case "var%array_comp%vec(1)" which
is supposed to have the rank of array_comp.
In fact I think the conditional supposed to support the CLASS cases is
completely bogus, and I don't see why the generic conditional just above
it wouldn't also support CLASS cases just fine.
Did you try removing the CLASS conditional entirely?

Mikael


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]