Page MenuHomePhabricator

Handle invalid types in the nullPointerConstant AST matcher
ClosedPublic

Authored by aaron.ballman on Jun 21 2020, 9:07 AM.

Details

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.

Diff Detail

Event Timeline

aaron.ballman created this revision.Jun 21 2020, 9:07 AM
steveire added inline comments.Jun 21 2020, 1:57 PM
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 marked an inline comment as done.Jun 22 2020, 4:06 AM
aaron.ballman added inline comments.
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 accepted this revision.Jun 22 2020, 3:14 PM
steveire added inline comments.
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
2618

Fair enough!

This revision is now accepted and ready to land.Jun 22 2020, 3:14 PM