[cfe-dev] [Patch] Update to Checker Development Manual

Anna Zaks ganna at apple.com
Wed Apr 10 22:08:01 PDT 2013

Sam, thank you so much for working on this!

Here are some comments. However, this generally looks good.


---- Custom Program states
+Checkers often need to extend the program state information to include their own
+custom information. The preferred way to do so is to use one of several macros
+designed for this purpose. They are:

Mention that checkers are stateless and the callbacks are not guaranteed to occur in a predetermined order, so checkers have to update the ProgramState to store their own state.

+<p>These macros will cover a majority of use cases; however, they still have a
+few limitations. They cannot be used inside namespaces (since they expand to
+contain top-level namespace references), and the data types that they define
+cannot be referenced from more than one file.

I would just say "Note that the macros cannot be used inside namespaces (since they expand to contain top-level namespace references)." The macros can be referred to from multiple files if they are defined in a header that is included from all of them. It's just that each checker is usually contained in a single file.

How about restructuring the section a bit to group the intro/explanation of each macro and their getters and setters together?

---- Checker Registration
We should refer to SimpleStreamChecker here. We should probably remove the checker class from the code since it will be described next in the Checker Skeleton section.

---- Checker Skeleton
- of events (steps during the analysis)
+ of events 
Otherwise, it's hard to read.

- a checker to check that file streams are used properly
+ a checker that warns about improper use of file stream APIs

- It can also be used to remove data that is no longer needed from the program state.
+ It must also be used to remove data that is no longer needed from the program state.

+ PreCall: Before each function call.
I do not like the structure of this description. Specifically, "Before each function call." is not a proper sentence.

Would be great if we could remove some whitespace from the code snippet; it looks very sparse.

---- Bug Reports
- so that it can be output. 
+ so that it can be displayed

- Where the bug is. 
+ Context of the bug.
(Not sure it's much better, but it is a noun as the rest of parameter descriptions)

+ and the program's state when this location is reached. This is in the form on an ExplodedNode.
Let's just say node in the ExplodedGraph, in which the error is reported. 

addTransition should be a link
Also, addTransition always has arguments.

When describing the difference between sinks and regular nodes used for error reporting, I'd mention that a sink is generated when continuing the exploration of the path after the error does not make sense, as the program is in an invalid state. For example, a null pointer dereference uses a sink, while leak does not.
I think the current flow does not make much sense anymore. For example, "Idea for a Checker" should go before the sections you've added. I think the following order would be best:

Getting Started
Static Analyzer Overview
  Interaction with Checkers
Idea for a Checker
Checker Registration
Checker Skeleton
Custom Program States
Representing Values // With added introduction saying that we are going to store info about the values we are observing into the state..
Bug Reports

What do you think? I don't mind working on restructuring myself after this content goes in..


On Mar 24, 2013, at 5:49 PM, Sam Handler <samuel.handler+cfedev at gmail.com> wrote:

> Hello all,
> Several months ago, I posted a tutorial about developing a plugin for the Analyzer. After some discussion, it was decided that parts of the tutorial should be integrated into the existing Checker Development Manual.
> I've finally had time to complete the majority of this work. Attached is a patch that contains the changes to date. This adds the following information:
>  - Custom Program States
>  - Checker Skeleton
>  - Bug Reports
>  - Additional sources of information
> I also cleaned up the table of contents a bit, and fixed a incorrect </a> tag.
> There are still some other parts that I am looking at adding, most notably a description of how the Analyzer processes through a function, building the program state (I had an example of this in the original tutorial, but I now think it will need to be completely rewritten). Since I'm not sure how long these additional sections will take, I want to get the ball rolling on getting these changes reviewed and merged.
> --Sam
> -- 
> ===============================================================================
> All opinions expressed in posts from this account are entirely my own, and not
> those of any present or former employer. Furthermore, I assert that all works
> contributed to the Clang project (1) were developed using no equipment,
> supplies, facility or trade secrets of any such employer, (2) were developed
> entirely on my own time, and (3) do not result from any work performed for any
> such employer.
> <checker_dev_manual.diff>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130410/b77795e6/attachment.html>

More information about the cfe-commits mailing list