Hi Thomas,

>> The string length expressions of overridden procedures have to be
>> identical, but with exchanged dummy arguments. Since the dummy
>> arguments of overridden procedures must have the same name as in the
>> base procedure, it is sufficient the check for equal names. Checking
>> for equal symbols would be too strict.
>
>
> I just tested the following patch:
>
> Index: dependency.c
> ===================================================================
> --- dependency.c        (Revision 177487)
> +++ dependency.c        (Arbeitskopie)
> @@ -123,7 +123,7 @@ gfc_are_identical_variables (gfc_expr *e1, gfc_exp
>  {
>   gfc_ref *r1, *r2;
>
> -  if (e1->symtree->n.sym != e2->symtree->n.sym)
> +  if (strcmp(e1->symtree->n.sym->name, e2->symtree->n.sym->name))
>     return false;
>
>   /* Volatile variables should never compare equal to themselves.  */
>
> without any regressions.  Can anybody think of a case where the names can be
> identical, but the variables different?  (I can't).

Well, I'd say this can only happen if both variables reside in
different namespaces (i.e. different modules or procedures).


> Maybe we can relax the test that way and get rid of the large number
> of changes for gfc_dep_compare_expr everywhere (which I confess I
> dislike, but I can hardly find fault with something that I have done
> only yesterday, although the number of changes was much smaller there :-)

Ok, I don't like the large number of changes either, but I assumed
they were necessary.

I have to admit I'm not aware of all the cases that
'gfc_dep_compare_expr' was intended for originally. I was only trying
to re-use it for checking overriding procedures, which seems to work
very well, except for the "variable names vs. symbols" issue. If you
tell me it's fine to only check for variable names everywhere, this is
of course fine.

Btw, the fact that your patch has no regressions does not necessarily
mean that there are no cases where it could fail. It could just mean
that the testsuite does not cover these cases.

Cheers,
Janus

Reply via email to