[Checkers] Checkers Code Review
jhp at csail.mit.edu
Sun Jun 22 21:49:39 EDT 2008
I started my review with the documentation. Attached are my comments.
Comments on the code to follow.
Note that I tried to read the documentation very carefully (as a
prospective checker writer). I included everything that I found
even slightly confusing (even if I later figured it out). Please
disregard anything that seems unimportant.
- Would it be possible to have a running example that could
be referred to? Much of the explanations are extremely
technical (at least to me) and I would have have trouble getting
started (without referring to an example).
- It seems that a fair amount of detailed knowledge of the javac compiler
internals is assumed. This may be necessary, but I think it may be
off-putting to someone considering writing a checker. If it were
possible to lessen this, I think it would be much more encouraging to
prospective checker writers. A running example, might also serve
to make this feel less daunting.
My guess is that its really pretty easy to write a simple checker,
but that is not the impression I get from reading the documentation.
I'd probably figure that out if I looked at one of the examples,
but I'd probably start with the documentation.
- in the 9.2 bullet, it might be helpful to include an example of
a literal (I presume that "any string" and java.lang.String.class
are examples). I mention this both because the term may not be
clear to everyone and I was confused because I tend to think of
literals as primitives (like 5 or 1.0) which don't really make
sense for nonnull.
- in the 9.3 bullet, are these rules in addition to those defined
in the hierarchy? Like the dereference of pointers that are not
nonnull? I think that is what is meant by '...your checker
automatically inherits such rules', but that wasn't clear to me.
- What is the @interface keyword?
- What is a meta-annotation?
- What does it mean to say that the framework 'only handles types with
one qualifier'? I'm guessing this means that there can only be
one qualifer on a single type (eg, it doesn't make sense to say
@nonnull @null String), not that there can only be one qualifier
in the type hierarchy, but on first reading that is not clear. If
i'm reading this correctly, it might make sense to downplay this as
I doubt anyone would expect to put two qualifiers (from the same
type hierarchy on a single instance of a type).
- Where are these annotations placed? The first line says on the
declaration of qualifier annotations. The last paragraph says
'on the checker class'. What is the checker class? I'm guessing
this is pretty simple and an example would make this clear. But
I really don't know what files I'm supposed to create and how they
interact with my code.
- What does the term 'invariant' mean in the next to last sentence?
I presume that it means that the generic's type arguments must be
exactly the same (not a subtype of one another). If this could be
accurately described without requiring readers to remember the
definitions of covariant, contravariant, and (I think, the less
common) invariant, I think that would make this more clear. I don't
think I'm the only one who has to look up these terms most times
I encounter them (it used to be every time, so I'm making progress).
Would it be inaccurate to say:
For instance, Java language specifications species that two
generic types are subtypes only if their type arguments are
identical. The Javari type system overrides this behavior to
allow a generic type to be a subclass of another generic if
the first generics type arguments are subtypes of the second
(e.g., List<@Mutable Date> is a subtype of List<@QReadOnly
- What do I need to know to understand this section? For example, is
there a list of applicable class literals for treeClasses?
I presume that what I must do is understand the compiler types
Tree.KIND, TypeKind, Tree, and TypeMirror. But if there is a simpler
way of figuring this out, it would certainly make this easier to
Perhaps understanding those classes in detail is essential for any
checker, but if not, it would be great if we could provide (or
reference) a list of reasonable choices and what they mean.
- After the bullets the text reads 'the Nullable annotation is annotated...',
Is this a meta-annotation?
- How does the nonnull checker specifiy all literals except null
(since I presume NULL_LITERAL) is a subtype of LiteralTree). Or is
this something that has to be specified in code?
- I think the third paragraph should begin with 'A checker's' rather than
'The checker's'. This certainly seems minor, but I find myself sometimes
confused about what a checker writer is to do (which I consider 'a'
checker) and something defined by the framework (possibly 'the' checker).
- Is the class 'TypeChecker' the class the checker writer creates that
extends BaseTypeChecker? I presume this is the case, but it is never
explicitly stated. Its also not in the code font, but is in the
mixed case format, so I'm not sure what it is intended to be. And
is 'FooChecker' an example of TypeChecker?
- What do I do in TypeChecker? Do I need do anything more than annotate
it with the TypeQualifiers annotation? If this is really simple,
an example would drive home how easy this is.
Michael Ernst wrote:
>> I updated the files in the same old location (~/mali/public_html/
>> checkers-code-review) to the latest checker code.
> Warning: checkers.pdf was not updated, but checkers.ps was.
More information about the checkers