[cfe-dev] Checker Plugins

[Argyrios Kyrtzidis]

On Aug 6, 2011, at 5:15 PM, Jordy Rose wrote:

> I think it's a good idea to keep these two concepts separate, but "registering" with the CheckerManager is the already-established term.

Well we we can always un-establish it :-)

> I was using "load" to mean "collect information about what checkers are available". Is there a better name for CheckerRegistry that captures the sense of "what checkers are available"? (Go back to "CheckerProvider"?)

Across llvm the "registry" concept is well established, I think we should follow the same term for "registering" available checkers and when actually creating concrete checker objects that the CheckerManager will use we can say this is "loading" or "initializing" or something else..

-Argyrios

> 
> Jordy
> 
> 
> On Aug 5, 2011, at 14:33, Argyrios Kyrtzidis wrote:
> 
>> Hey Jordy,
>> 
>> Thanks for your work!
>> 
>> As a general comment I think there should a be clear separation (naming and functionality) between the concept of "registering" checkers (information of available checkers is put into the CheckerRegistry) and "loading" of checkers (checker instances are created and put into CheckerManager). "Registering" will happen first, then "loading" will depend on CheckerRegistry and CheckersControlList.
>> 
>> If you agree, please do related changes like:
>> 
>> void loadBuiltinCheckers(CheckerRegistry &registry);  -> void registerBuiltinCheckers(CheckerRegistry &registry);
>> 
>> typedef void (*RegistrationFunction)(CheckerManager &);  ->  typedef void (*LoadFunction)(CheckerManager &);
>> 
>> allCheckers.registerCheckers(*checkerMgr, checkerOpts);  ->  allCheckers.loadCheckers(*checkerMgr, checkerOpts);
>> 
>> checkerMgr->finishedCheckerRegistration();   ->  checkerMgr->finishedCheckerLoading();
>> 
>> etc..
>> 
>> And ento::registerCheckers combines the two concepts; I'd suggest separating into a registerCheckers function to fill out a CheckerRegistry that will get used for loading or help printing.
>> 
>> Let me know what you think.
>> 
>> -Argyrios
>> 
>> On Aug 4, 2011, at 8:29 PM, Jordy Rose wrote:
>> 
>>> Whoops, forgot to include the removal of CheckerProvider from CheckerManager.cpp.
>>> 
>>> <CheckerPlugins.4.patch>
>>> 
>>> 
>>> On Aug 4, 2011, at 19:01, Jordy Rose wrote:
>>> 
>>>> Here's my latest work of CheckerPlugin, as per off-list discussion on what checker registration should look like. This time I haven't included any dynamic loading yet, only switched the current CheckerProvider interface over to a more useful CheckerRegistry. Plugin checker registration, based on a single per-library entry point (clang_getCheckers?), should pretty much not affect what's in this design at all.
>>>> 
>>>> The actual plugin loading code is blocked by the DynamicLibrary patch I sent to llvm-commits a few days ago.
>>>> 
>>>> There's a lot of infrastructure that might seem like premature optimization (in particular, keeping around the size of every package to avoid string comparisons when selecting checkers). This was mostly me freaking out about the speed hit on "make test" until I realized that my baseline was wrong. (I recently started building with CXX=clang++, Ted rewrote the CFG traversal, speed of a Debug+Asserts build isn't representative anyway...whatever.) So I can go ahead and take all that out again.
>>>> 
>>>> What do you think?
>>>> Jordy
>>>> 
>>>> <CheckerPlugins.4.patch>
>>> 
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> 
> 
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev