[Checkers] Framework Change Request

[Matt Papi]

Interestingly, I was just thinking about this when addressing the
performance issues for NonNull. Here's what I opted for instead --
it's very low impact (in terms of patch size) but also probably less
effective (in terms of performance boost) than what you've proposed:

- add a cache to AnnotatedTypeFactory that maps Trees to TreePaths
- override BaseTypeVisitor.scan so that it puts (tree,
getCurrentPath()+tree) into the cache
- add a factory.getPath method that checks the cache and on miss uses
(+ caches the result of) Trees.getPath()  (arguably this method
doesn't belong in AnnotatedTypeFactory)
- use factory.getPath instead of Trees.getPath

I'm using it to address issues 3 and 5 in NonNullAnnotatedTypeFactory
and it works pretty well (NonNull is about twice as fast on Daikon).


I really like your idea better from a design standpoint, though it
might be a pain to use in some places -- we're not always calling
getAnnotatedType() in a place where the path is readily available
(TypeFromTree?), and sometimes even in visitors we call it on a child
of the current path (e.g., node.getExpression() in visitMemberSelect),
in which case the path needs to be extended. (I'm actually less
concerned about us having to make these changes, and more concerned
about people that are trying to read/understand the existing checkers
or write their own.)

 - Matt

On Wed, Apr 23, 2008 at 12:27 PM, Mahmood Ali <mahmood at mit.edu> wrote:
> Hi guys,
>
>  As I am reviewing the framework and saw the dependancies on
>  TreePath.getPath() function, I realized that in many cases we do want
>  the context of the assignment:
>  1. is l-value: this-mutable in Javari resolves as mutable as lvalue,
>  but readonly as rvalue
>     I noticed that Flow has a function to determine whether it's an
>  lvalue
>  2. self-type: need to figure out the most enclosing method and class
>  3. jdk-bug work-around: want to check whether a parameter is a catch
>  exception parameter or just a regular parameter.
>  4. type variable resolution: want to see what the assignment context is
>  5. default anotation: we want to get the nearest default annotation.
>
>  To address all of these issues, we have been using TreePath.getPath()
>  (or a variation of it).  We stepped through hoops with VisitorState to
>  reduce such calls.  However, the visitor already has the path (through
>  getCurrentPath()).
>
>  I am proposing (what Mike suggested sometime before) to supply that
>  expression context (through TreePath) to getAnnotatedType(), making
>  the signature of AnnotatedTypeFactory.getAnnotatedType(Tree) into
>  getAnnotatedType(TreePath).  This makes out API closer to the Javac
>  Tree API, as it has Trees.getTypeMirror(TreePath path).
>
>  In the last time I ran the profiler, TreePath.getPath() was using
>  about 50% of time for nonnull checker because of default annotation.
>
>  Any suggestions?
>
>  - Mahmood
>
>  P.S. For the jdk-bug workaround, I made it check whether it is a catch
>  exception parameter only if the type is throwable which saved some
>  time, but the depenedancy on getPath() is still there.
>
>
>
>  _______________________________________________
>  checkers mailing list
>  checkers at lists.csail.mit.edu
>  https://lists.csail.mit.edu/mailman/listinfo/checkers
>