Author: Yitzhak Mandelbaum Date: 2020-02-28T13:46:27-05:00 New Revision: 586f13aeac3fd51916774f523e30f0aee2b62d46
URL: https://github.com/llvm/llvm-project/commit/586f13aeac3fd51916774f523e30f0aee2b62d46 DIFF: https://github.com/llvm/llvm-project/commit/586f13aeac3fd51916774f523e30f0aee2b62d46.diff LOG: [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 Added: Modified: clang/lib/ASTMatchers/ASTMatchersInternal.cpp clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Removed: ################################################################################ diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp index 777400f84974..6f4132c19aed 100644 --- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -347,12 +347,18 @@ 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); + } } - *Builder = std::move(Result); + // 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; } diff --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp index 65fb11caa489..edfd1c3fbd91 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp @@ -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; };", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
