On Friday 19 August 2011 23:54:45 Janus Weil wrote: > > 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. OK, this comment really bugged me, but it's not that bad on second thought.
> > > 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. Ah yes. I was thinking expressions themselves were compared; but only their values are. > One could try to handle such cases in a follow-up patch. If you want. I wasn't asking you (or anyone else) to do it. > > I'll commit the patch (as posted) tomorrow, if Mikael agrees that the > description is ok. It's fine. Thanks. Mikael.
