[cfe-commits] Checker and respondsToCallback

Jordy Rose jediknil at belkadan.com
Thu Aug 5 22:27:41 PDT 2010

Checkers aren't currently stateless, but they probably ought to be much
more so, especially for external inlining.

Anyway, since Assume triggers ProcessAssume, we already have a problem --
many many checkers use Assume. We could take the optimization out of these
"subordinate" checks, but that sort of defeats the purpose.

Maybe CheckerContext needs to be a hierarchy. We could make the base
context very small (perhaps just the flag) and add to it for the various
callbacks. It's a little silly because it's sort of shadowing the
callbacks, but it could solve two problems: having a local context for
invocation to store the respondsToCallback flag, and having a common
signature for all the callbacks, to allow us to package the
logic in its own method(s). (A possible solution to the problem discussed
on another thread.)

On Thu, 5 Aug 2010 21:18:21 -0700, Ted Kremenek <kremenek at apple.com>
> I think it's fair to require that Checker callbacks can't call each
> If they need to call shared logic, they can use an internal, non-virtual
> function.
> I think the SaveAndRestore is also not necessary in ResetCallbackFlag. 
> GRExprEngine cares about the callback flag, it will reset it before
> the callback.  There is no need to restore it.
> One disadvantage that I realized about putting this flag in Checker is
> that it makes checkers less thread-safe.  I'm not certain what direction
> are heading it, but if Checkers were mostly stateless they could
> be used by multiple GRExprEngines in multiple threads.  From this
> perspective, putting the flag in the CheckerContext object is better.
> On Aug 5, 2010, at 12:14 PM, Jordy Rose 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
>> 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
>>> 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.
>> <Checker-respondsToCallback.patch>

More information about the cfe-commits mailing list