ymandel created this revision.
ymandel added reviewers: sbenza, aaron.ballman.
Herald added a project: clang.
Currently, `optionally` can take multiple arguments, which commits it to a
particular strategy for those arguments (in this case, "for each"). We limit the
matcher to a single argument, which avoids any potential confusion and
simplifies the implementation. The user can retrieve multiple-argument
optionality, by explicitly using the desired operator (like `forEach`, `anyOf`,
`allOf`, etc.) with all children wrapped in `optionally`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75556
Files:
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===================================================================
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2007,9 +2007,8 @@
TEST(Optionally, SubmatchersDoNotMatch) {
EXPECT_TRUE(matchAndVerifyResultFalse(
"class A { int a; int b; };",
- recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
- has(fieldDecl(hasName("d")).bind("v")))),
- std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));
+ recordDecl(optionally(has(fieldDecl(hasName("c")).bind("c")))),
+ std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("c")));
}
// Regression test.
@@ -2028,14 +2027,8 @@
TEST(Optionally, SubmatchersMatch) {
EXPECT_TRUE(matchAndVerifyResultTrue(
"class A { int a; int c; };",
- recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")),
- has(fieldDecl(hasName("b")).bind("v")))),
- std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 1)));
- EXPECT_TRUE(matchAndVerifyResultTrue(
- "class A { int c; int b; };",
- recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
- has(fieldDecl(hasName("b")).bind("v")))),
- std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 2)));
+ recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")))),
+ std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));
}
TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -346,18 +346,11 @@
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
ArrayRef<DynTypedMatcher> InnerMatchers) {
- BoundNodesTreeBuilder Result;
- bool Matched = false;
- for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
- BoundNodesTreeBuilder BuilderInner(*Builder);
- 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)
+ if (InnerMatchers.size() != 1)
+ return false;
+
+ BoundNodesTreeBuilder Result(*Builder);
+ if (InnerMatchers[0].matches(DynNode, Finder, &Result))
*Builder = std::move(Result);
return true;
}
@@ -859,9 +852,8 @@
const internal::VariadicOperatorMatcherFunc<
2, std::numeric_limits<unsigned>::max()>
allOf = {internal::DynTypedMatcher::VO_AllOf};
-const internal::VariadicOperatorMatcherFunc<
- 1, std::numeric_limits<unsigned>::max()>
- optionally = {internal::DynTypedMatcher::VO_Optionally};
+const internal::VariadicOperatorMatcherFunc<1, 1> optionally = {
+ internal::DynTypedMatcher::VO_Optionally};
const internal::VariadicFunction<internal::Matcher<NamedDecl>, StringRef,
internal::hasAnyNameFunc>
hasAnyName = {};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2612,9 +2612,7 @@
/// member named "bar" in that class.
///
/// Usable as: Any Matcher
-extern const internal::VariadicOperatorMatcherFunc<
- 1, std::numeric_limits<unsigned>::max()>
- optionally;
+extern const internal::VariadicOperatorMatcherFunc<1, 1> optionally;
/// Matches sizeof (C99), alignof (C++11) and vec_step (OpenCL)
///
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===================================================================
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2007,9 +2007,8 @@
TEST(Optionally, SubmatchersDoNotMatch) {
EXPECT_TRUE(matchAndVerifyResultFalse(
"class A { int a; int b; };",
- recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
- has(fieldDecl(hasName("d")).bind("v")))),
- std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));
+ recordDecl(optionally(has(fieldDecl(hasName("c")).bind("c")))),
+ std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("c")));
}
// Regression test.
@@ -2028,14 +2027,8 @@
TEST(Optionally, SubmatchersMatch) {
EXPECT_TRUE(matchAndVerifyResultTrue(
"class A { int a; int c; };",
- recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")),
- has(fieldDecl(hasName("b")).bind("v")))),
- std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 1)));
- EXPECT_TRUE(matchAndVerifyResultTrue(
- "class A { int c; int b; };",
- recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
- has(fieldDecl(hasName("b")).bind("v")))),
- std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 2)));
+ recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")))),
+ std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));
}
TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -346,18 +346,11 @@
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
ArrayRef<DynTypedMatcher> InnerMatchers) {
- BoundNodesTreeBuilder Result;
- bool Matched = false;
- for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
- BoundNodesTreeBuilder BuilderInner(*Builder);
- 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)
+ if (InnerMatchers.size() != 1)
+ return false;
+
+ BoundNodesTreeBuilder Result(*Builder);
+ if (InnerMatchers[0].matches(DynNode, Finder, &Result))
*Builder = std::move(Result);
return true;
}
@@ -859,9 +852,8 @@
const internal::VariadicOperatorMatcherFunc<
2, std::numeric_limits<unsigned>::max()>
allOf = {internal::DynTypedMatcher::VO_AllOf};
-const internal::VariadicOperatorMatcherFunc<
- 1, std::numeric_limits<unsigned>::max()>
- optionally = {internal::DynTypedMatcher::VO_Optionally};
+const internal::VariadicOperatorMatcherFunc<1, 1> optionally = {
+ internal::DynTypedMatcher::VO_Optionally};
const internal::VariadicFunction<internal::Matcher<NamedDecl>, StringRef,
internal::hasAnyNameFunc>
hasAnyName = {};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2612,9 +2612,7 @@
/// member named "bar" in that class.
///
/// Usable as: Any Matcher
-extern const internal::VariadicOperatorMatcherFunc<
- 1, std::numeric_limits<unsigned>::max()>
- optionally;
+extern const internal::VariadicOperatorMatcherFunc<1, 1> optionally;
/// Matches sizeof (C99), alignof (C++11) and vec_step (OpenCL)
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
