Handle invalid types in the nullPointerConstant AST matcher

[aaron.ballman]

Summary

Currently, using the nullPointerConstant AST matcher can lead to assertions in situations where a node to be matched does not have a valid type associated with it, such as a ParenListExpr. This patch addresses that by saying such nodes cannot be a null pointer constant. This addresses PR46353.

[steveire]

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2618

While this test in ASTMatchers might continue to make sense, shouldn't there be a test in clang/unittests/AST for this API? isNullPointerConstant seems to be complicated. Does it have a unit test?

[aaron.ballman]

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2618

While this test in ASTMatchers might continue to make sense, shouldn't there be a test in clang/unittests/AST for this API? isNullPointerConstant seems to be complicated. Does it have a unit test?

It does not have a unit test; we don't usually unit test AST interfaces directly. AST matching tests are the typical way we'd test this because it gives us an almost direct path to the AST interface we want to exercise, but if you can spot an additional place to perform the test more directly, I can look into adding one.

[steveire]

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2618

Fair enough!

[aaron.ballman]

Thanks for the review! Committed in 8a9311940a26372dab6706edfd07288667394cfe

clang/lib/AST/Expr.cpp

Expr::isNullPointerConstant(ASTContext &Ctx,
   } else if (const MaterializeTemporaryExpr *M
                                    = dyn_cast<MaterializeTemporaryExpr>(this)) {
     return M->getSubExpr()->isNullPointerConstant(Ctx, NPC);
   } else if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(this)) {
     if (const Expr *Source = OVE->getSourceExpr())
       return Source->isNullPointerConstant(Ctx, NPC);
   }
 
+  // If the expression has no type information, it cannot be a null pointer
+  // constant.
+  if (getType().isNull())
+    return NPCK_NotNull;
+
   // C++11 nullptr_t is always a null pointer constant.
   if (getType()->isNullPtrType())
     return NPCK_CXX11_nullptr;
 
   if (const RecordType *UT = getType()->getAsUnionType())
     if (!Ctx.getLangOpts().CPlusPlus11 &&
         UT && UT->getDecl()->hasAttr<TransparentUnionAttr>())
       if (const CompoundLiteralExpr *CLE = dyn_cast<CompoundLiteralExpr>(this)){

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

 TEST(NullPointerConstants, Basic) {
   EXPECT_TRUE(matches("#define NULL ((void *)0)\n"
                         "void *v1 = NULL;", expr(nullPointerConstant())));
   EXPECT_TRUE(matches("void *v2 = nullptr;", expr(nullPointerConstant())));
   EXPECT_TRUE(matches("void *v3 = __null;", expr(nullPointerConstant())));
   EXPECT_TRUE(matches("char *cp = (char *)0;", expr(nullPointerConstant())));
   EXPECT_TRUE(matches("int *ip = 0;", expr(nullPointerConstant())));
   EXPECT_TRUE(matches("int i = 0;", expr(nullPointerConstant())));
+  const char kTest[] = R"(
+    template <typename T>
+    struct MyTemplate {
+      MyTemplate() : field_(__null) {}
+      T* field_;
+    };
+  )";
+  EXPECT_TRUE(matches(kTest, expr(nullPointerConstant())));
 }
 
 TEST(HasExternalFormalLinkage, Basic) {
   EXPECT_TRUE(matches("int a = 0;", namedDecl(hasExternalFormalLinkage())));
   EXPECT_TRUE(
       notMatches("static int a = 0;", namedDecl(hasExternalFormalLinkage())));
   EXPECT_TRUE(notMatches("static void f(void) { int a = 0; }",
                          namedDecl(hasExternalFormalLinkage())));