[cfe-dev] fix for Clang PR 8419

[Zhanyong Wan (λx.x x)]

On Thu, Nov 18, 2010 at 4:17 PM, Zhanyong Wan (λx.x x) <wan at google.com> wrote:
> On Thu, Nov 18, 2010 at 3:31 PM, Zhanyong Wan (λx.x x) <wan at google.com> wrote:
>> I'm not sure this is the case.  Given code:
>>
>> class Foo {
>>  public:
>>  char& get() const;
>> };
>>
>> char& get();
>>
>> void Test() {
>>  Foo foo;
>>  foo.get()++;
>>  get()++;  // Crashes.
>> }
>>
>> 'clang --analyze' has no trouble with "foo.get()++" but crashes on
>> "get()++", so the culprit seems to be in how CallExpr (as opposed to
>> CXXMethodCallExpr) is handled.
>>
>> While debugging this, I saw one thing that I don't understand:
>>
>> GRExprEngine::ProcessStmt() calls Visit() as opposed to VisitLValue()
>> when processing the "foo.get()" subexpression of "foo.get()++".
>>  Is this right or a bug?  My understanding is that "foo.get()" is an
>> L-value and thus should be handled by VisitLValue() -- what am I
>> missing?  Thanks,
>
> Now I see that ProcessStmt() actually processes "foo.get()" twice.
> The first time it's treated as a top-level expression, and Visit() is
> used.  (That's what I talked about in my previous message.)  The
> second time it's treated as a sub-expression of "foo.get()++", and
> VisitLValue() is used (more precisely, Visit("foo.get()++) is called,
> which then calls VisitLValue("foo.get()")).  This makes more sense
> now.  Though it's still unclear to me why the first time is needed.

Here's my theory of why "foo.get()" is processed before "foo.get()++":

The static analyzer executes the code symbolically by following the
control flow.  When

  foo.get()++;

is executed, first "foo.get()" needs to be evaluated, as dictated by
C++'s semantics.  Hence ProcessStmt() processes "foo.get()" first.
Next, "++" is done on the result of "foo.get()".  This causes
ProcessStmt() to process "foo.get()++" as a whole.  Even though the
call Visit("foo.get()++") looks to me like it's simulating the
execution of the entire "foo.get()++" expression, it actually only
simulates the "top-level" part of it -- that is, "x++" where "x" is
the result of "foo.get()".

Is my understanding correct?  Thanks,

-- 
Zhanyong