[ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

[ymandel]

Summary

This matcher combines the features of the hasParent matcher with
skip-implicit-expression-nodes behavior of ignoringImplicit. It finds the
first ancestor which is reachable through only implict expression nodes and
matches the given argument matcher.

[ymandel]

update dynamic registry and the ast matcher doc.

[ymandel]

Fixed to use more standard type adaptors. Registration now works.

[aaron.ballman]

This seems to be strongly related to the TK_IgnoreUnlessSpelledInSource traversal behavior; is there a reason you can't use that traversal mode with hasParent() (does it behave differently)?

[ymandel]

In D88275#2295379, @aaron.ballman wrote:

This seems to be strongly related to the TK_IgnoreUnlessSpelledInSource traversal behavior; is there a reason you can't use that traversal mode with hasParent() (does it behave differently)?

Aaron, that's a good point. I hadn't realized that the traversal mode supported parent traversal. That said, even with it available, I have strong reservations about modal matching, especially given that it had severe issues when TK_IgnoreUnlessSpelledInSource went live a few months ago as the default setting. I don't know what the resolution of those discussions were, but I thought we simply agreed to restore the default, and left fixing the underlying issues as future work.

But, even it if worked as desired, to achieve the same effect, you would have to restore the current mode as well once you're reached your desired destination. e.g. inspired by the tests, given some matcher m,

integerLiteral(hasParentIgnoringImplicit(varDecl(m)))

becomes

integerLiteral(traversal(TK_IgnoreUnlessSpelledInSource, hasParent(varDecl(traversal(TK_AsIs, m)))))

which seems a lot worse to me. We could however implement this new one in terms of traverse, but that would go back to the question of whether it is working correctly and also gets somewhat tricky (specifically, restoring the ambient traversal mode). Do you know the current status?

thanks

[ymandel]

restored changes to unrelated parts of html docs.

[aaron.ballman]

In D88275#2295413, @ymandel wrote:

In D88275#2295379, @aaron.ballman wrote:

This seems to be strongly related to the TK_IgnoreUnlessSpelledInSource traversal behavior; is there a reason you can't use that traversal mode with hasParent() (does it behave differently)?

Aaron, that's a good point. I hadn't realized that the traversal mode supported parent traversal. That said, even with it available, I have strong reservations about modal matching, especially given that it had severe issues when TK_IgnoreUnlessSpelledInSource went live a few months ago as the default setting. I don't know what the resolution of those discussions were, but I thought we simply agreed to restore the default, and left fixing the underlying issues as future work.

That is the resolution we came to.

But, even it if worked as desired, to achieve the same effect, you would have to restore the current mode as well once you're reached your desired destination. e.g. inspired by the tests, given some matcher m,

integerLiteral(hasParentIgnoringImplicit(varDecl(m)))

becomes

integerLiteral(traversal(TK_IgnoreUnlessSpelledInSource, hasParent(varDecl(traversal(TK_AsIs, m)))))

which seems a lot worse to me.

It's certainly more wordy, but do you envision this matcher functionality to be needed so often that we need to introduce a second way to achieve the same thing?

We could however implement this new one in terms of traverse, but that would go back to the question of whether it is working correctly and also gets somewhat tricky (specifically, restoring the ambient traversal mode). Do you know the current status?

I'm adding @steveire in case he has any updates or opinions, but my understanding of the current status is that the defaults have been restored but no further work has been done for implicit node matching.

I'm not concerned about the basic idea behind the proposed matcher, I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

[ymandel]

I'm not concerned about the basic idea behind the proposed matcher, I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

Aaron, I appreciate this concern, but I would argue that this matcher isn't making things any worse. We already have the various ignoringImplicit matchers, and this new one simply parallels those, but for parents. So, it is in some sense "completing" an existing API, which together is an alternative to traverse.

We should decide our general policy on these implict matchers vs the traverse matchers. Personally, I view traverse as an experimental feature that we're still figuring out and so would prefer that it not block progress on new matchers. But, I'm open to discussion. I can implement this one in my own codebase in the meantime if this requires longer discussion (that is, it's not blocking me, fwiw).

Also, I don't believe that traverse work in this case. When I change the test to use traverse, it fails:

TEST(HasParentIgnoringImplicit, TraverseMatchesExplicitParents) {
  std::string Input = R"cc(
    float f() {
        int x = 3;
        int y = 3.0;
        return y;
    }
  )cc";
   // ---> Passes because there are no implicit parents.
  EXPECT_TRUE(matches(
      Input, integerLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                     hasParent(varDecl())))));
  // Fails
  EXPECT_TRUE(
      matches(Input, declRefExpr(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                          hasParent(returnStmt())))));
  // Fails
  EXPECT_TRUE(
      matches(Input, floatLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                           hasParent(varDecl())))));
}

[aaron.ballman]

In D88275#2303283, @ymandel wrote:

I'm not concerned about the basic idea behind the proposed matcher, I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

Aaron, I appreciate this concern, but I would argue that this matcher isn't making things any worse. We already have the various ignoringImplicit matchers, and this new one simply parallels those, but for parents. So, it is in some sense "completing" an existing API, which together is an alternative to traverse.

I'm not certain I agree with that reasoning because you can extend it to literally any match that may interact with implicit nodes, which is the whole point to the spelled in source traversal mode. I'm not certain it's a good design for us to continue to add one-off ways to ignore implicit nodes.

We should decide our general policy on these implict matchers vs the traverse matchers.

I agree.

Personally, I view traverse as an experimental feature that we're still figuring out and so would prefer that it not block progress on new matchers. But, I'm open to discussion. I can implement this one in my own codebase in the meantime if this requires longer discussion (that is, it's not blocking me, fwiw).

FWIW, I think of traverse as experimental as well. I can see the argument for not letting an experimental feature block progress on new matchers, too. I'm mostly worried about the case where we add the new matches and keep the traverse API, but they have subtly different behaviors and no clear policy on when to use which form.

Also, I don't believe that traverse work in this case. When I change the test to use traverse, it fails:

TEST(HasParentIgnoringImplicit, TraverseMatchesExplicitParents) {
  std::string Input = R"cc(
    float f() {
        int x = 3;
        int y = 3.0;
        return y;
    }
  )cc";
   // ---> Passes because there are no implicit parents.
  EXPECT_TRUE(matches(
      Input, integerLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                     hasParent(varDecl())))));
  // Fails
  EXPECT_TRUE(
      matches(Input, declRefExpr(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                          hasParent(returnStmt())))));
  // Fails
  EXPECT_TRUE(
      matches(Input, floatLiteral(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                           hasParent(varDecl())))));
}

Interesting catch and not the behavior I would expect from traverse. @steveire, would you consider this to be a bug in the traversal behavior?

[ymandel]

In D88275#2305989, @aaron.ballman wrote:

In D88275#2303283, @ymandel wrote:

I'm not concerned about the basic idea behind the proposed matcher, I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

Aaron, I appreciate this concern, but I would argue that this matcher isn't making things any worse. We already have the various ignoringImplicit matchers, and this new one simply parallels those, but for parents. So, it is in some sense "completing" an existing API, which together is an alternative to traverse.

I'm not certain I agree with that reasoning because you can extend it to literally any match that may interact with implicit nodes, which is the whole point to the spelled in source traversal mode. I'm not certain it's a good design for us to continue to add one-off ways to ignore implicit nodes.

I appreciate your point, but I'm personally inclined to allow progress while we figure these larger issues out. That said, I'm in no rush and would like a solution that we're both happy with. How do you propose we proceed, especially given that traverse does not currently support this case? It seems that progress is in part blocked on hearing back from steveire, but it's been over a week since you added him to the review thread. Is there some other way to ping him?

Thanks!

[aaron.ballman]

In D88275#2313342, @ymandel wrote:

In D88275#2305989, @aaron.ballman wrote:

In D88275#2303283, @ymandel wrote:

I'm not concerned about the basic idea behind the proposed matcher, I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

Aaron, I appreciate this concern, but I would argue that this matcher isn't making things any worse. We already have the various ignoringImplicit matchers, and this new one simply parallels those, but for parents. So, it is in some sense "completing" an existing API, which together is an alternative to traverse.

I'm not certain I agree with that reasoning because you can extend it to literally any match that may interact with implicit nodes, which is the whole point to the spelled in source traversal mode. I'm not certain it's a good design for us to continue to add one-off ways to ignore implicit nodes.

I appreciate your point, but I'm personally inclined to allow progress while we figure these larger issues out. That said, I'm in no rush and would like a solution that we're both happy with. How do you propose we proceed, especially given that traverse does not currently support this case? It seems that progress is in part blocked on hearing back from steveire, but it's been over a week since you added him to the review thread. Is there some other way to ping him?

Thank you for your patience while we sort this out. I've pinged @steveire off-list, so hopefully he'll respond with his thoughts. If we don't hear from Stephen in the next week or so, I think we should proceed with your proposed patch to get you unblocked (adding one more "ignore implicit" variant isn't the end of the world, I just want us to be thoughtful about whether we want to add new matchers in this space or to work on the traversal mode instead).

[ymandel]

In D88275#2316484, @aaron.ballman wrote:

Thank you for your patience while we sort this out. I've pinged @steveire off-list, so hopefully he'll respond with his thoughts. If we don't hear from Stephen in the next week or so, I think we should proceed with your proposed patch to get you unblocked (adding one more "ignore implicit" variant isn't the end of the world, I just want us to be thoughtful about whether we want to add new matchers in this space or to work on the traversal mode instead).

Sure! Thanks for reaching out to Stephen. This sounds good to me.

[steveire]

I don't get email notifications when I'm pinged on Phab for some reason. I didn't know about this until the email from Aaron.

// Fails
EXPECT_TRUE(
    matches(Input, declRefExpr(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                        hasParent(returnStmt())))));

Why do you use TK_IgnoreImplicitCastsAndParentheses instead of TK_IgnoreUnlessSpelledInSource? All you seem to be doing is showing that the former is broken. I've always thought we should remove the former. AsIs and IgnoreUnlessSpelledInSource should be enough for anyone.

@steveire, would you consider this [when using TK_IgnoreImplicitCastsAndParentheses] to be a bug in the traversal behavior?

It's probably a bug in how TK_IgnoreImplicitCastsAndParentheses is processed?

I have strong reservations about modal matching, especially given that it had severe issues

I think "severe" is an overstatement.

I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

The traverse matcher and IgnoreUnlessSpelledInSource were introduced so that matchers like hasParentIgnoringImplicit (and all the other hasParentIgnoring*) would never be needed (and so that the already-existing ignore* matchers would be needed only rarely). I agree with Aaron that it's not a good design to continue.

I think the existence of this new hasParentIgnoringImplicit matcher is further motivation that IgnoreUnlessSpelledInSource should be used more, especially when writing new matcher code. It is simpler. I get the impression people haven't tried it and prefer to write the complicated stuff instead.

I still don't see a problem with traverse being modal, but that fact seems to make you not use it, @ymandel ?

[ymandel]

TL;DR Stephen's fix works; I'll drop this patch.

Why do you use TK_IgnoreImplicitCastsAndParentheses instead of TK_IgnoreUnlessSpelledInSource? All you seem to be doing is showing that the former is broken. I've always thought we should remove the former. AsIs and IgnoreUnlessSpelledInSource should be enough for anyone.

@steveire, would you consider this [when using TK_IgnoreImplicitCastsAndParentheses] to be a bug in the traversal behavior?

It's probably a bug in how TK_IgnoreImplicitCastsAndParentheses is processed?

Changing to TK_IgnoreUnlessSpelledInSource fixes the test, as you suggested. Thanks!

As for why I chose the former -- it was the closest match to what I was doing and the comments on the struct didn't guide me one way or another.

I have strong reservations about modal matching, especially given that it had severe issues

I think "severe" is an overstatement.

I think it was an unhelpful word choice -- sorry about that. I could go into more detail as to why I chose that, but without detail it's not a helpful description. Suffice it to say that we had multiple instances of suprising behaviour, which lost quite a few hours.

I'm only worried we're making AST matching more confusing by having two different ways of inconsistently accomplishing the same-ish thing.

The traverse matcher and IgnoreUnlessSpelledInSource were introduced so that matchers like hasParentIgnoringImplicit (and all the other hasParentIgnoring*) would never be needed (and so that the already-existing ignore* matchers would be needed only rarely). I agree with Aaron that it's not a good design to continue.

I think the existence of this new hasParentIgnoringImplicit matcher is further motivation that IgnoreUnlessSpelledInSource should be used more, especially when writing new matcher code. It is simpler. I get the impression people haven't tried it and prefer to write the complicated stuff instead.

I still don't see a problem with traverse being modal, but that fact seems to make you not use it, @ymandel ?

This is a longer discussion and not necessarily worth having on this review thread. Are you attending the dev meeting today? If so, want to chat during the coffee break? Otherwise, it seems like it could be helpful to schedule a meeting to discuss for some other time.

That said, I'm more inclined towards its use as a specialty operator, for situations like this, than as a general purpose tool for matchers. I can't say I much like the current regime of explicit ignore operators either.

[steveire]

In D88275#2319290, @ymandel wrote:

TL;DR Stephen's fix works; I'll drop this patch.

This is a longer discussion and not necessarily worth having on this review thread. Are you attending the dev meeting today? If so, want to chat during the coffee break? Otherwise, it seems like it could be helpful to schedule a meeting to discuss for some other time.

I didn't go to the coffee break, but am in the afters hangout.

clang/docs/LibASTMatchersReference.html

 Usable as: Any Matcher
 Note that has is direct matcher, so it also matches things like implicit
 casts and paren casts. If you are matching with expr then you should
 probably consider using ignoringParenImpCasts like:
 has(ignoringParenImpCasts(expr())).
 </pre></td></tr>
 
 
+
+<tr><td>Matcher&lt;*&gt;</td><td class="name" onclick="toggle('hasParentIgnoringImplicit0')"><a name="hasParentIgnoringImplicit0Anchor">hasParentIgnoringImplicit</a></td><td>Matcher&lt;*&gt;</td></tr>
+<tr><td colspan="4" class="doc" id="hasParentIgnoringImplicit0"><pre>Matches Stmt nodes that have an ancestor, reachable only through
+implicit-expression nodes, that matches the provided matcher.
+
+Given
+float f() { return 3; }
+integerLiteral(hasParentIgnoringImplicit(returnStmt())) matches "3",
+while integerLiteral(hasParent(returnStmt())) does not.
+
+Usable as: Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;
+</pre></td></tr>
+
 <tr><td>Matcher&lt;*&gt;</td><td class="name" onclick="toggle('hasParent0')"><a name="hasParent0Anchor">hasParent</a></td><td>Matcher&lt;*&gt;</td></tr>
 <tr><td colspan="4" class="doc" id="hasParent0"><pre>Matches AST nodes that have a parent that matches the provided
 matcher.
 
 Given
 void f() { for (;;) { int x = 42; if (true) { int x = 43; } } }
 compoundStmt(hasParent(ifStmt())) matches "{ int x = 43; }".
 

 </pre></td></tr>
 
 <!--END_TRAVERSAL_MATCHERS -->
 </table>
 
 </div>
 </body>
 </html>
-
-

clang/include/clang/ASTMatchers/ASTMatchers.h

 ///
 /// Usable as: Any Matcher
 extern const internal::ArgumentAdaptingMatcherFunc<
     internal::HasParentMatcher,
     internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>,
     internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>>
     hasParent;
 
+/// Matches \c Stmt nodes that have an ancestor, reachable only through
+/// implicit-expression nodes, that matches the provided matcher.
+///
+/// Given
+/// \code
+/// float f() { return 3; }
+/// \endcode
+/// \c integerLiteral(hasParentIgnoringImplicit(returnStmt())) matches "3",
+/// while \c integerLiteral(hasParent(returnStmt())) does not.
+///
+/// Usable as: Matcher<Stmt>
+extern const internal::ArgumentAdaptingMatcherFunc<
+    internal::HasParentIgnoringImplicitMatcher,
+    internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>,
+    internal::TypeList<Stmt>>
+    hasParentIgnoringImplicit;
+
 /// Matches AST nodes that have an ancestor that matches the provided
 /// matcher.
 ///
 /// Given
 /// \code
 /// void f() { if (true) { int x = 42; } }
 /// void g() { for (;;) { int x = 43; } }
 /// \endcode

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

public:
   };
 
   /// Defines which ancestors are considered for a match.
   enum AncestorMatchMode {
     /// All ancestors.
     AMM_All,
 
     /// Direct parent only.
-    AMM_ParentOnly
+    AMM_ParentOnly,
+
+    /// Considers the first non-implicit `Expr` ancestor. Intuitively, like
+    /// `ignoringImplicit` for matching parents.
+    AMM_FirstExplicitOnly
   };
 
   virtual ~ASTMatchFinder() = default;
 
   /// Returns true if the given C++ class is directly or indirectly derived
   /// from a base type matching \c base.
   ///
   /// A class is not considered to be derived from itself.
public:
 
   bool matches(const T &Node, ASTMatchFinder *Finder,
                BoundNodesTreeBuilder *Builder) const override {
     return Finder->matchesAncestorOf(Node, this->ParentMatcher, Builder,
                                      ASTMatchFinder::AMM_ParentOnly);
   }
 };
 
+/// Matches \c Stmt nodes that have an ancestor note that is separated from
+/// `Node` only by implicit expression nodes and matches `ParentMatcher`. We
+/// limit to \c Stmt because implicit expressions only ever contain statements
+/// (expressions, in particular).
+///
+/// \c ParentT must be an AST base type.
+template <typename T, typename ParentT>
+class HasParentIgnoringImplicitMatcher : public MatcherInterface<T> {
+  static_assert(IsBaseType<ParentT>::value,
+                "has-parent-ignoring-implicit only accepts base type matcher");
+  static_assert(std::is_base_of<Stmt, T>::value,
+                "has-parent-ignoring-implict only matches statements");
+
+  const DynTypedMatcher ParentMatcher;
+
+public:
+  explicit HasParentIgnoringImplicitMatcher(
+      const Matcher<ParentT> &ParentMatcher)
+      : ParentMatcher(ParentMatcher) {}
+
+  bool matches(const T &Node, ASTMatchFinder *Finder,
+               BoundNodesTreeBuilder *Builder) const override {
+    return Finder->matchesAncestorOf(Node, this->ParentMatcher, Builder,
+                                     ASTMatchFinder::AMM_FirstExplicitOnly);
+  }
+};
+
 /// Matches nodes of type \c T that have at least one ancestor node of
 /// type \c AncestorT for which the given inner matcher matches.
 ///
 /// \c AncestorT must be an AST base type.
 template <typename T, typename AncestorT>
 class HasAncestorMatcher : public MatcherInterface<T> {
   static_assert(IsBaseType<AncestorT>::value,
                 "has ancestor only accepts base type matcher");

clang/lib/ASTMatchers/ASTMatchFinder.cpp

public:
   bool matchesAncestorOf(const DynTypedNode &Node, ASTContext &Ctx,
                          const DynTypedMatcher &Matcher,
                          BoundNodesTreeBuilder *Builder,
                          AncestorMatchMode MatchMode) override {
     // Reset the cache outside of the recursive call to make sure we
     // don't invalidate any iterators.
     if (ResultCache.size() > MaxMemoizationEntries)
       ResultCache.clear();
-    if (MatchMode == AncestorMatchMode::AMM_ParentOnly)
+    switch (MatchMode) {
+    case AncestorMatchMode::AMM_ParentOnly:
       return matchesParentOf(Node, Matcher, Builder);
+    case AncestorMatchMode::AMM_FirstExplicitOnly:
+      return matchesFirstExplicitAncestorOf(Node, Matcher, Builder);
+    case AncestorMatchMode::AMM_All:
       return matchesAnyAncestorOf(Node, Ctx, Matcher, Builder);
     }
+  }
 
   // Matches all registered matchers on the given node and calls the
   // result callback for every node that matches.
   void match(const DynTypedNode &Node) {
     // FIXME: Improve this with a switch or a visitor pattern.
     if (auto *N = Node.get<Decl>()) {
       match(*N);
     } else if (auto *N = Node.get<Stmt>()) {
for (const auto &Parent : ActiveASTContext->getParents(Node)) {
       if (Matcher.matches(Parent, this, &BuilderCopy)) {
         *Builder = std::move(BuilderCopy);
         return true;
       }
     }
     return false;
   }
 
+  // Returns whether the first explicit (`Expr`) ancestor of \p Node matches \p
+  // Matcher. That is, like matchesParentOf but skipping implicit parents.
+  // Unlike matchesAnyAncestorOf there's no memoization: it doesn't save much.
+  bool matchesFirstExplicitAncestorOf(const DynTypedNode &Node,
+                                      const DynTypedMatcher &Matcher,
+                                      BoundNodesTreeBuilder *Builder) {
+    for (const auto &Parent : ActiveASTContext->getParents(Node)) {
+      if (const auto *E = Parent.get<clang::Expr>())
+        // If the parent is an implicit node, match on *its* parents
+        // instead. Use DFS, since we expect that expressions are relatively
+        // shallow.
+        if (clang::isa<ImplicitCastExpr>(E) || clang::isa<FullExpr>(E) ||
+            clang::isa<MaterializeTemporaryExpr>(E) ||
+            clang::isa<CXXBindTemporaryExpr>(E)) {
+          if (matchesFirstExplicitAncestorOf(Parent, Matcher, Builder))
+            return true;
+          continue;
+        }
+      BoundNodesTreeBuilder BuilderCopy = *Builder;
+      if (Matcher.matches(Parent, this, &BuilderCopy)) {
+        *Builder = std::move(BuilderCopy);
+        return true;
+      }
+    }
+    return false;
+  }
+
   // Returns whether an ancestor of \p Node matches \p Matcher.
   //
   // The order of matching (which can lead to different nodes being bound in
   // case there are multiple matches) is breadth first search.
   //
   // To allow memoization in the very common case of having deeply nested
   // expressions inside a template function, we first walk up the AST, memoizing
   // the result of the match along the way, as long as there is only a single

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasName);
   REGISTER_MATCHER(hasNullSelector);
   REGISTER_MATCHER(hasObjectExpression);
   REGISTER_MATCHER(hasOperands);
   REGISTER_MATCHER(hasOperatorName);
   REGISTER_MATCHER(hasOverloadedOperatorName);
   REGISTER_MATCHER(hasParameter);
   REGISTER_MATCHER(hasParent);
+  REGISTER_MATCHER(hasParentIgnoringImplicit);
   REGISTER_MATCHER(hasQualifier);
   REGISTER_MATCHER(hasRHS);
   REGISTER_MATCHER(hasRangeInit);
   REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
   REGISTER_MATCHER(hasPlacementArg);

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

integerLiteral(equals(42),
                            hasParent(functionDecl(hasParent(cxxRecordDecl(
                              unless(isTemplateInstantiation())))))))))));
   EXPECT_TRUE(
     notMatches("template <typename T> struct C { static void f() {} };"
                  "void t() { C<int>::f(); }",
                compoundStmt(hasParent(recordDecl()))));
 }
 
+TEST(HasParentIgnoringImplicit, MatchesExplicitParents) {
+  std::string Input = R"cc(
+    float f() {
+        int x = 3;
+        int y = 3.0;
+        return y;
+    }
+  )cc";
+  EXPECT_TRUE(
+      matches(Input, declRefExpr(hasParentIgnoringImplicit(returnStmt()))));
+  EXPECT_TRUE(
+      matches(Input, floatLiteral(hasParentIgnoringImplicit(varDecl()))));
+  EXPECT_TRUE(
+      matches(Input, integerLiteral(hasParentIgnoringImplicit(varDecl()))));
+
+  // Make sure it only ignores implicit ancestors.
+  EXPECT_TRUE(
+      notMatches(Input, integerLiteral(hasParentIgnoringImplicit(declStmt()))));
+}
+
 TEST(HasParent, NoDuplicateParents) {
   class HasDuplicateParents : public BoundNodesCallback {
   public:
     bool run(const BoundNodes *Nodes) override { return false; }
     bool run(const BoundNodes *Nodes, ASTContext *Context) override {
       const Stmt *Node = Nodes->getNodeAs<Stmt>("node");
       std::set<const void *> Parents;
       for (const auto &Parent : Context->getParents(*Node)) {