>> > 2011/8/7 Thomas Koenig <tkoe...@netcologne.de>: >> >> When extending the values of gfc_dep_compare_expr, we will need to go >> >> through all its uses (making sure we change == -2 to <= -2). >> > >> > attached is a patch which makes a start with this. >> > >> > For now, it changes the return value to "-3" for two cases: >> > 1) different expr_types >> > 2) non-identical variables >> > >> > I tried to take care of all places which are checking for a return >> > value of "-2" and I hope I missed none. >> > >> > Any objections or ok for trunk? (Regtested successfully.) > OK from my side for the code proper.
Thanks for the review. > I have one comment though about this: > +/* Compare two expressions. Return values: > + * +1 if e1 > e2 > + * 0 if e1 == e2 > + * -1 if e1 < e2 > + * -2 if the relationship could not be determined > + * -3 if e1 /= e2, but we cannot tell which one is larger. */ > > I think this is misleading, as the function does not always return -3 when > e1/=e2. That's right. However, the same argument applies to the other values as well: The function does not always return 0 if e1==e2. There could be cases where the arguments are algebraically equal, but we fail to detect this (example: A+B+C vs C+B+A). This sort of "uncertainty" was not introduced by me, but was present before, and is not special to the value "-3". Describing the value -2 as "relationship could not be determined" sort of implies that this can happen. So I would tend to leave the description as it is. > There is for example (currently) no special handling for operators. Well, unfortunately one cannot just return "-3" for non-matching operators. Just think of cases like A*(B+C) vs A*B+A*C. One could try to handle such cases in a follow-up patch. I'll commit the patch (as posted) tomorrow, if Mikael agrees that the description is ok. Also I like Tobias' idea of using an enum, but I'll leave it for a follow-up. Cheers, Janus
