Submitter | Thomas Koenig |
---|---|

Date | Aug. 6, 2011, 3:39 p.m. |

Message ID | <4E3D601A.7020401@netcologne.de> |

Download | mbox | patch |

Permalink | /patch/108789/ |

State | New |

Headers | show |

## Comments

On Saturday 06 August 2011 17:39:06 Thomas Koenig wrote: > > 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. In the PR at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49638#c8 I quote the standard: 4.5.7.3 (type-bound procedure overriding) has: • Either both shall be subroutines or both shall be functions having the same result characteristics (12.3.3). 12.3.3 (Characteristics of function results): If a type parameter of a function result or a bound of a function result array is not a constant expression, the exact dependence on the entities in the expression is a characteristic So the standards is more restrictive than expression values being the same. It requires _the exact same dependence on the entities_. My reading of this is that 3*(x+1) vs 3*x+3 is right to be rejected, same for (a+b)+c vs a+(b+c). The only worrying case that I see is the one you pointed out: a+b+c vs c+b+a (without brackets). Mikael

>> 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. > In the PR at > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49638#c8 > I quote the standard: > > 4.5.7.3 (type-bound procedure overriding) has: > • Either both shall be subroutines or both shall be functions having the same > result characteristics (12.3.3). > > 12.3.3 (Characteristics of function results): > If a type parameter of a function result or a bound of a function result array > is not a constant expression, the > exact dependence on the entities in the expression is a characteristic > > > So the standards is more restrictive than expression values being the same. It > requires _the exact same dependence on the entities_. My reading of this is > that 3*(x+1) vs 3*x+3 is right to be rejected, same for (a+b)+c vs a+(b+c). > The only worrying case that I see is the one you pointed out: a+b+c vs c+b+a > (without brackets). Huh, I don't see what is so different between 1) 3*(x+1) vs 3*x+3 and 2) a+b+c vs c+b+a In both cases the expressions look different at first sight, but can be transformed into each other mathematically. So I'd say they are mathematically equivalent, although the spelled-out representations of these expressions differ. The question is how you interpret the standard's formulation of "exact dependence on the entities in the expression". Naively I would have taken this to mean the *mathematical* dependence (which can be represented by different actual expressions). But I'm fine with your interpretation, too, which will make life even easier for us. Cheers, Janus

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

Am 06.08.2011 18:16, schrieb Janus Weil: >> 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). > gfc_are_identical variables is only called from within gfc_dep_compare_expr. It makes no sense to call this function to compare expressions from different statements, unless one has carefully analyzed that no intervening assignment to the variables has taken place. Comparing across namespaces makes even less sense. So yes, I think it is enough if we compare the variable names, and document this in a commtent. > 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. Well, I also wrote the function, so maybe I can claim a little bit of authority here on the way it was originally meant to be ;-) > 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. Of course. However, when analysis and regression test agree, it is usually a good sign :-)

On Saturday 06 August 2011 18:06:58 Janus Weil wrote: > >> 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. > > > > In the PR at > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49638#c8 > > I quote the standard: > > > > 4.5.7.3 (type-bound procedure overriding) has: > > • Either both shall be subroutines or both shall be functions having the > > same result characteristics (12.3.3). > > > > 12.3.3 (Characteristics of function results): > > If a type parameter of a function result or a bound of a function result > > array is not a constant expression, the > > exact dependence on the entities in the expression is a characteristic > > > > > > So the standards is more restrictive than expression values being the > > same. It requires _the exact same dependence on the entities_. My > > reading of this is that 3*(x+1) vs 3*x+3 is right to be rejected, same > > for (a+b)+c vs a+(b+c). The only worrying case that I see is the one you > > pointed out: a+b+c vs c+b+a (without brackets). > > Huh, I don't see what is so different between > > 1) 3*(x+1) vs 3*x+3 and > 2) a+b+c vs c+b+a > > In both cases the expressions look different at first sight, but can > be transformed into each other mathematically. So I'd say they are > mathematically equivalent, although the spelled-out representations of > these expressions differ. I was looking at the standard, because I was not so sure myself. Here is what is written (7.1.5.2.4): Once the interpretation of a numeric intrinsic operation is established, the processor may evaluate any mathematically equivalent expression, provided that the integrity of parentheses is not violated. Two expressions of a numeric type are mathematically equivalent if, for all possible values of their primaries, theirmathematical values are equal. So parentheses have to be respected; other than that anything is possible. This is about the evaluation of expressions though, not about the "dependences on entities". > > The question is how you interpret the standard's formulation of "exact > dependence on the entities in the expression". That is the question. > Naively I would have > taken this to mean the *mathematical* dependence (which can be > represented by different actual expressions). But I'm fine with your > interpretation, too, which will make life even easier for us. Yes, my interpretation is somewhat biased towards ease of implementation. ;-) Mikael

>>> 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). >> > > gfc_are_identical variables is only called from within gfc_dep_compare_expr. > It makes no sense to call this function > to compare expressions from different statements, unless one has carefully > analyzed that no intervening assignment to the variables has taken place. > Comparing across namespaces makes even less sense. > > So yes, I think it is enough if we compare the variable names, and > document this in a commtent. Actually, on second thought, I disagree. For the original usage cases of gfc_dep_compare_expr, I'm not sure if one can guarantee the expressions to be in the same namespace. However, for the overriding checks, both expressions are guaranteed to be in *different* namespaces (namely: two different procedures). And as Mikael noted, it is crucial wether the symbols in the expressions are dummy arguments or not: 1) Dummies are guaranteed to have equal names in overridden procedures, so we can just compare names. 2) Non-dummies could have the same name, but still sit in different namespaces, so for them we really have to check for equal symbols! Here is a variant of the original test case from the PR, which will be accepted if we only check for names (but it should actually be rejected): module world implicit none type :: world_1 contains procedure, nopass :: string => w1_string end type type, extends(world_1) :: world_2 contains procedure, nopass :: string => w2_string end type contains function w1_string (m) integer, parameter :: n = 5 integer, intent(in) :: m character(n+m) :: w1_string w1_string = "world" end function function w2_string (m) integer, parameter :: n = 6 integer, intent(in) :: m character(n+m) :: w2_string w2_string = "world2" end function end module Cheers, Janus

> Here is a variant of the original test case from the PR, which will be > accepted if we only check for names (but it should actually be > rejected): > > > module world > > implicit none > > type :: world_1 > contains > procedure, nopass :: string => w1_string > end type > > type, extends(world_1) :: world_2 > contains > procedure, nopass :: string => w2_string > end type > > contains > > function w1_string (m) > integer, parameter :: n = 5 > integer, intent(in) :: m > character(n+m) :: w1_string > w1_string = "world" > end function > > function w2_string (m) > integer, parameter :: n = 6 > integer, intent(in) :: m > character(n+m) :: w2_string > w2_string = "world2" > end function > > end module Sorry, now I have to disagree with my own earlier claims: In this example, the 'n' variables will of course be simplified to EXPR_CONSTANTs, so the name checking does not apply to them. And since the string length can not depend on local variables which are *not* constant, name checking should still be fine! Now, if Thomas says it's fine for the other cases, too, then it seems we can really get away with a much simpler patch. Hope we're not missing anything, though ... Cheers, Janus

On Saturday 06 August 2011 19:10:09 Janus Weil wrote: > Now, if Thomas says it's fine for the other cases, too, then it seems > we can really get away with a much simpler patch. Hope we're not > missing anything, though ... > What about this case: two module variables from two different modules? module world1 implicit none integer :: n type :: t1 contains procedure, nopass :: string => w1_string end type contains function w1_string (m) integer, intent(in) :: m character(n) :: w1_string w1_string = "world" end function end module world1 module world2 use world1, only : t1 implicit none integer :: n type, extends(t1) :: t2 contains procedure, nopass :: string => w2_string end type contains function w2_string (m) integer, intent(in) :: m character(n) :: w2_string w2_string = "world2" end function end module world2

On Sat, Aug 06, 2011 at 06:45:36PM +0200, Mikael Morin wrote: > On Saturday 06 August 2011 18:06:58 Janus Weil wrote: > > >> 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. > > > > > > In the PR at > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49638#c8 > > > I quote the standard: > > > > > > 4.5.7.3 (type-bound procedure overriding) has: > > > ? Either both shall be subroutines or both shall be functions having the > > > same result characteristics (12.3.3). > > > > > > 12.3.3 (Characteristics of function results): > > > If a type parameter of a function result or a bound of a function result > > > array is not a constant expression, the > > > exact dependence on the entities in the expression is a characteristic > > > > > > > > > So the standards is more restrictive than expression values being the > > > same. It requires _the exact same dependence on the entities_. My > > > reading of this is that 3*(x+1) vs 3*x+3 is right to be rejected, same > > > for (a+b)+c vs a+(b+c). The only worrying case that I see is the one you > > > pointed out: a+b+c vs c+b+a (without brackets). > > > > Huh, I don't see what is so different between > > > > 1) 3*(x+1) vs 3*x+3 and > > 2) a+b+c vs c+b+a > > > > In both cases the expressions look different at first sight, but can > > be transformed into each other mathematically. So I'd say they are > > mathematically equivalent, although the spelled-out representations of > > these expressions differ. > I was looking at the standard, because I was not so sure myself. > Here is what is written (7.1.5.2.4): > > Once the interpretation of a numeric intrinsic operation is established, the > processor may evaluate any mathematically equivalent expression, provided that > the integrity of parentheses is not violated. > > Two expressions of a numeric type are mathematically equivalent if, for all > possible values of their primaries, theirmathematical values are equal. > > > So parentheses have to be respected; other than that anything is possible. See Note 7.18. X*(Y-Z) -> X*Y - X*Z is a forbidden transformation (there is no noted restriction on Z > 0). a + b + c -> b + a + c -> b + (a + c) is a sequence of allowable transformations.

## 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. */