Commit 586f13ae authored by Yitzhak Mandelbaum's avatar Yitzhak Mandelbaum
Browse files

[AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings... 

[AST Matchers] Fix bug in 'optionally' matcher wherein all previous bindings are cleared when all inner matchers fail.

Summary: The implementation of 'optionally' doesn't preserve bindings when none of the submatchers succeed. This patch adds a regression test for that behavior and fixes it.

Reviewers: aaron.ballman, sbenza

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75365
parent 1de10705

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

+8 −2
Original line number Diff line number Diff line
@@ -347,11 +347,17 @@ bool OptionallyVariadicOperator(const DynTypedNode &DynNode, 
                                BoundNodesTreeBuilder *Builder,

                                ArrayRef<DynTypedMatcher> InnerMatchers) {

  BoundNodesTreeBuilder Result;

  bool Matched = false;

  for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {

    BoundNodesTreeBuilder BuilderInner(*Builder);

    if (InnerMatcher.matches(DynNode, Finder, &BuilderInner))

    if (InnerMatcher.matches(DynNode, Finder, &BuilderInner)) {

      Matched = true;

      Result.addMatch(BuilderInner);

    }

  }

  // If there were no matches, we can't assign to `*Builder`; we'd (incorrectly)

  // clear it because `Result` is empty.

  if (Matched)

    *Builder = std::move(Result);

  return true;

}

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

+13 −0
Original line number Diff line number Diff line
@@ -2012,6 +2012,19 @@ TEST(Optionally, SubmatchersDoNotMatch) { 
      std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));

}



// Regression test.

TEST(Optionally, SubmatchersDoNotMatchButPreserveBindings) {

  std::string Code = "class A { int a; int b; };";

  auto Matcher = recordDecl(decl().bind("decl"),

                            optionally(has(fieldDecl(hasName("c")).bind("v"))));

  // "decl" is still bound.

  EXPECT_TRUE(matchAndVerifyResultTrue(

      Code, Matcher, std::make_unique<VerifyIdIsBoundTo<RecordDecl>>("decl")));

  // "v" is not bound, but the match still suceeded.

  EXPECT_TRUE(matchAndVerifyResultFalse(

      Code, Matcher, std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));

}



TEST(Optionally, SubmatchersMatch) {

  EXPECT_TRUE(matchAndVerifyResultTrue(

      "class A { int a; int c; };",