[cfe-commits] Checker and respondsToCallback

Jordy Rose jediknil at belkadan.com
Thu Aug 5 18:35:44 PDT 2010


I was adhering strictly to information-hiding -- once we're at this point,
it's little better than just accessing respondsToCallback directly. But it
is simpler. I think I went overboard.


On Fri, 6 Aug 2010 09:31:17 +0800, Zhongxing Xu <xuzhongxing at gmail.com>
wrote:
> Why can't we just do that:
> 
> in Checker.h:
> 
>   bool &getCallbackFlag() {
>     return respondsToCallback;
>   }
> 
> In GRExprEngine.cpp:
> 
> +    SaveAndRestore<bool> OldFlag(checker->getCallbackFlag(), true);
> 
> 
> On Fri, Aug 6, 2010 at 9:11 AM, Zhongxing Xu <xuzhongxing at gmail.com>
wrote:
>> If I understand correctly, we can add an assertion here, and remove
>> the argument 'true'.
>>
>> +  SaveAndRestore<bool> ResetCallbackFlag() {
>> +    // This looks like it's incorrect, since a non-optimizing compiler
>> will
>> +    // use a temporary and a copy here, then destroy the copy and
reset
>> the
>> +    // value of respondsToCallback. However, the only case where
>> +    // respondsToCallback is 'false' is just after calling an ignored
>> +    // callback. By the end of the block in which a client calls that
>> callback,
>> +    // the value of respondsToCallback will have been reset to 'true'.
>>
>>     assert(respondsToCallback);
>>
>> +    return SaveAndRestore<bool>(respondsToCallback, true);
>> +  }
>>
>> On Fri, Aug 6, 2010 at 3:14 AM, Jordy Rose <jediknil at belkadan.com>
wrote:
>>> Hm. Thanks to the fact that one callback may end up indirectly
invoking
>>> another, this is not so simple. On the one hand, it does let us track
>>> the
>>> response for any callback. On the other, ResetCallbackFlag is a
kludge.
>>>
>>> Holding off on committing for now.
>>>
>>>
>>> On Wed, 4 Aug 2010 17:10:17 -0700, Ted Kremenek <kremenek at apple.com>
>>> wrote:
>>>> That's not a bad idea at all.  In fact I really like it.  The value
>>>> can
>>>> still get lazily set, but just stored with the Checker object.
>>>>
>>>> On Aug 4, 2010, at 10:51 AM, Jordy Rose wrote:
>>>>
>>>>> This is not a general solution, though. Alternately, we could just
>>> stick
>>>>> the flag in Checker rather than CheckerContext.
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>



More information about the cfe-commits mailing list