Page MenuHomePhabricator

[ASTMatcher] Add CXXNewExpr support to hasDeclaration
ClosedPublic

Authored by malcolm.parsons on Oct 27 2016, 6:15 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

malcolm.parsons retitled this revision from to [ASTMatcher] Add operatorNew matcher for cxxNewExpr.
malcolm.parsons added a subscriber: cfe-commits.
aaron.ballman added a subscriber: sbenza.
aaron.ballman added inline comments.
include/clang/ASTMatchers/ASTMatchers.h
4042 ↗(On Diff #76014)

Instead of adding a new traversal matcher for operatorNew, we could use the existing hasDeclaration traversal matcher and provide another overload for it.

The CXXNewExpr does expose the selected operator delete overload as well, so I'm not certain using hasDeclaration is perfect, but it was the first traversal matcher I could think of when trying to go from a new expression to the overloaded operator new declaration chosen for the expression (which is similar to CallExpr usage). @klimek or @sbenza, do you have a preference?

Extend hasDeclaration instead.

malcolm.parsons retitled this revision from [ASTMatcher] Add operatorNew matcher for cxxNewExpr to [ASTMatcher] Add CXXNewExpr support to hasDeclaration.Oct 29 2016, 1:24 PM
aaron.ballman added a subscriber: lukasza.

I think this is the right way to go, but I know that @klimek and @lukasza have been working on hasDeclaration() issues recently, so I am wondering what their thoughts are as well.

lukasza edited edge metadata.Oct 31 2016, 9:54 AM

FWIW, a non-owner LGTM:

  • CXXNewExpr seems very similar to CallExpr, so it makes sense that hasDeclaration would behave similarily for both of these expressions (i.e. matching the "callee")
  • The issues we've been trying to work through in https://reviews.llvm.org/D24361 mainly revolve around Type and QualType, so I think those issues should not apply to CXXNewExpr matching.
aaron.ballman accepted this revision.Oct 31 2016, 1:55 PM
aaron.ballman edited edge metadata.

Assuming @klimek agrees with the design, this LGTM. If you don't hear from Manuel by next Wed, I think you're okay to commit (we can always roll it back post-commit if needed).

This revision is now accepted and ready to land.Oct 31 2016, 1:55 PM
klimek accepted this revision.Oct 31 2016, 2:05 PM
klimek edited edge metadata.

Yep, makes sense. Open issues are all about types :)

This revision was automatically updated to reflect the committed changes.