CFRefCount "Problem" #3: Debug-printing ProgramStates

[Background: Currently we're trying to eliminate the concept of "transfer functions" from the static analyzer, which has gradually been supplanted by the Checker interface. The only implementation of the transfer functions, CFRefCount, has two roles: general-purpose function and method evaluator, and retain-count checking for the Cocoa and CoreFoundation frameworks. The former is being moved to ExprEngine*, the latter to a regular checker called RetainReleaseChecker. Of course, there are always design issues; I'm trying to make sure they get cleared up on-list one by one.]

This one is barely a problem: currently there's a notion of a "state printer" for ProgramStates, which is intended for debug use in printing the contents of a state. However, only TransferFuncs gets a shot at adding new printers.

Now that we /have/ the checker infrastructure, it makes more sense to me to have a new callback, print::State, which allows checkers to print any checker-specific data...such as RetainReleaseChecker's RefBindings and autorelease pool contents, and perhaps PthreadLockChecker's locksets or CStringChecker's string lengths...and ditch the notion of a separate "program state printer" owned by the ProgramStateManager.

This seems pretty straightforward to me, though there might be details to change?

printState.patch (15 KB)

Hi Jordy,

I like this cleanup. When looking at the patch, I'm not certain why we need to have a _registerForPrintState, like we do for other callbacks. Why not make this a virtual member function in CheckerBase?

The reason we have the registerXXX logic for Stmt callbacks is because they are in the fast path of the analyzer. We only want to callback to the checkers during analysis that actually implement a given callback (and in the case where there are no checkers with callbacks for a given StmtKind, we do no extra work). We could have just declared a ton of virtual functions for all pre-/post- statement callbacks, but this would have been slow, as we would have had to loop over every single checker every time we visited a Stmt.

State printing is part of debugging. It's not in the hot path, and I don't see a reason why we just don't use a virtual function here. That would significantly reduce a bunch of logic in the patch.

What do you think?

Hm, good point. Case of "it's always been done that way" (which isn't even true). Sort of too bad we still have to jump through ExprEngine to get to CheckerManager, but that's acceptable.