On Oct 29, 2009, at 10:49 AM, Daniel Dunbar wrote:
> I find this confusing, we now have two notions of "has side effects"
> in ExprConstant.cpp. Do these need to be distinct? At the least I'm
> surprised the visitor doesn't reuse the EvalResult one.
The existing one doesn't answer the question, does this expression
have any side effects. :-( I was wishing it did. For that reason, I
can't use it. I didn't see any common ground between them.
> Also, please include some test cases which exercise this stuff.
I have one in the testsuite, disguised as a _builtin_object_size
test... which was the main motivation for doing up the code. I
initially was just going to return true and put a FIXME to implement
it, but I thought, oh well, let's just pound it all out now and not
have a fixme. I'll see about adding some more.
>> + bool VisitArraySubscriptExpr(ArraySubscriptExpr *E)
>> + { return Visit(E->getLHS()) && Visit(E->getRHS()); }
>
> This should be || ?
Yup, good catch. Thanks.
>> + bool VisitBinaryOperator(BinaryOperator *E) { return false; }
>
> This needs to visit the sub expressions?
Yes.
>> + bool VisitUnaryDeref(UnaryOperator *E) {
>> + if (E->getType().isVolatileQualified())
>> + return true;
>> + return false;
>> + }
>
> So does this?
Yes, thanks.
These points fixed in r85526.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
