Hi Janus,
2011/8/5 Mikael Morin<mikael.mo...@sfr.fr>:
On Friday 05 August 2011 23:02:33 Thomas Koenig wrote:
The extra
argument controls whether we check variable symbols for equality or
just their names. For the overriding checks it is sufficient to check
for names, because the arguments of the overriding procedure are
required to have the same names as in the base procedure.
Could you explain for which cases this test is too strict?
For dummy arguments. If they are "corresponding" (same position, same name),
they should compare equal. Cf the PR.
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).
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 :-)
1) I have moved 'check_typebound_override' to interface.c and prefixed
it with 'gfc_'.
OK.
2) I have added the 'var_name_only flag' also to
gfc_are_identical_variables, gfc_dep_compare_functions,
identical_array_ref, check_section_vs_section and gfc_is_same_range. I
hope there is nothing else I missed.
See above; could we avoid that?
3) I have made 'gfc_are_identical_variables' static and removed the
gfc prefix (it does not seem to be used outside of dependency.c).
OK.
4) I have made 'gfc_is_same_range' static and removed the gfc prefix
(there is only a commented out reference to it in trans-array.c, so I
commented out the declaration in dependency.h, too). Also I removed
the 'def' argument, which gets always passed a '0'.
OK.
As Thomas mentions, certain cases are still not handled correctly
(e.g. A+B+C vs C+B+A, and other mathematical transformations), but I
hope they are sufficiently exotic (so that we can wait for bug reports
to roll in). In addition I expect people to declare overridden
procedures analogously to the base procedure, and not use e.g.
len=3*(x+1) in one case and len=3*x+3 in the other.
Not OK.
It is wrong to assume that expressions are unequal because we cannot
prove they are equal, with all the limitations that we currently
have. This will introduce rejects-valid bugs.
Please change
+ /* Check string length. */
+ if (proc_target->result->ts.type == BT_CHARACTER
+ && proc_target->result->ts.u.cl && old_target->result->ts.u.cl
+ && gfc_dep_compare_expr (proc_target->result->ts.u.cl->length,
+ old_target->result->ts.u.cl->length,
+ true) != 0)
to something like (untested)
+ /* Check string length. */
+ if (proc_target->result->ts.type == BT_CHARACTER
+ && proc_target->result->ts.u.cl && old_target->result->ts.u.cl
+ {
+ int len_comparision;
+ len_comparison = gfc_dep_compare_expr
(proc_target->result->ts.u.cl->length,
+ old_target->result->ts.u.cl->length);
+ if (len_comparison != 0 && len_comparison != -2)
...
Alternatively, you could raise an error for 1 and -1 and warn only for
-2 (... may be different).
Regards
Thomas
