[cfe-dev] Help with APValue comparisons

[Eli Friedman]

On Fri, Jun 15, 2012 at 7:01 AM, Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:
> On Friday, June 15, 2012 9:38 AM, Andy Gibbs wrote:
>
>> Hi Jordan,
>>
>> Thanks for your pointers.  I've implemented an "isEqual" function as
>> below. If you have time, would you mind having a quick skim through please
>> to see if I've caught the right idea.  At the moment, it holds up to the
>> testing I have performed so far, but I'm always working on more
>> test-cases!
>>
>> At this point, my concern is not that isEqual may not fully distinguish
>> two APValues that *are* equal, more that it should never say two APValues
>> are equal when they aren't.  False negatives are much less of an issue
>> than false positives!
>
> Ok, so please ignore my implementation for LValue in my previous post: I'm
> not sure exactly what I was thinking, but it was garbage!
>
> Here is a simplified and more correct implementation:
>
> case LValue: {
>  LValueBase BaseA = A.getLValueBase();
>  LValueBase BaseB = B.getLValueBase();
>
>  // null pointers on either side are not comparable
>  if (!BaseA || !BaseB || BaseA.isNull() || BaseB.isNull()) return false;
>
>  // simple comparison checks
>  if ((A.getLValueOffset()       != B.getLValueOffset())       ||
>      (A.getLValueCallIndex()    != B.getLValueCallIndex())    ||
>      (A.isLValueOnePastTheEnd() != B.isLValueOnePastTheEnd()) ||
>      (A.hasLValuePath()         != B.hasLValuePath())         ||
>      (BaseA.getOpaqueValue()    != BaseB.getOpaqueValue()))
>    return false;
>
>  // walk the path
>  if (A.hasLValuePath()) {
>    assert(B.hasLValuePath() && "this should not happen"); // checked above
>
>    ArrayRef<LValuePathEntry> PathA = A.getLValuePath();
>    ArrayRef<LValuePathEntry> PathB = B.getLValuePath();
>    if (PathA.size() != PathB.size()) return false;
>
>    for (unsigned I = 0, N = PathA.size(); I != N; ++I) {
>      if ((PathA[I].BaseOrMember != PathB[I].BaseOrMember) ||
>          (PathA[I].ArrayIndex   != PathB[I].ArrayIndex))
>        return false;
>    }
>  }
>
>  return true;
> }

We already have some code for comparing pointers in ExprConstant.cpp
(I think in IntExprEvaluator::visitBinaryOperator?); please take a
look at that.

The most obvious issue here is that the code doesn't seem to allow for
the possibility that we can't tell whether two LValues are equal.

-Eli