[PATCH] D17034: Add an AST matcher for null pointers

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:4821
@@ +4820,3 @@
+///   initializer for i.
+AST_MATCHER(Expr, nullPointerConstant) {
+  return Matcher<Expr>(
----------------
sbenza wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > sbenza wrote:
> > > > sbenza wrote:
> > > > > Use AST_MATCHER_FUNCTION instead, where the return value is the matcher (instead of the application of the matcher).
> > > > > It is simpler to write and since it has no arguments it will memoize the matcher and construct it only once.
> > > > Maybe use Expr::isNullPointerConstant?
> > > Ah, interesting! I hadn't known about that macro. Thank you.
> > Oh, hey, that's easier still! Would I still use AST_MATCHER_FUNCTION in that case though, or should that remain a simple AST_MATCHER?
> One or the other.
> The _FUNCTION macro is for when the matcher itself is just a matcher expression. Saves typing and CPU.
> Generating the temporary matchers on each match is expensive as they do memory allocation.
> If you use Expr::isNullPointerConstant you won't be making any matchers.
> 
One interesting problem I ran into with using Expr::isNullPointerConstant is that it makes the matcher overly aggressive with nullptr. Given:
```
AST_MATCHER(Expr, nullPointerConstant) {
  QualType QT = Node.getType();
  if (QT->isNullPtrType())
    return true;

  return QT->isPointerType() &&
         Node.isNullPointerConstant(Finder->getASTContext(),
                                    Expr::NPC_NeverValueDependent);
}
```
If you attempt to match `somePtr = nullptr;` with `expr(nullPointerConstant())`, you will get two matches. One for the initializer expression of the declaration, and one for the implicit cast expression. However, with `somePtr = __null` or `somePtr = 0`, you only get a single match (for the initializer expression). This seems like surprising behavior, but I'm not quite certain of the best way to address it, if it should be addressed at all.


http://reviews.llvm.org/D17034