steveire accepted this revision. steveire added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2618 + )"; + EXPECT_TRUE(matches(kTest, expr(nullPointerConstant()))); } ---------------- aaron.ballman wrote: > steveire wrote: > > 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? > > 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. Fair enough! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82279/new/ https://reviews.llvm.org/D82279 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
