[AST Matchers] Restrict `optionally` matcher to a single argument.

Summary:
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`.

Reviewers: sbenza, aaron.ballman

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75556
This commit is contained in:
Yitzhak Mandelbaum 2020-03-03 15:04:08 -05:00
parent 8d7b118875
commit c359f9537f
4 changed files with 21 additions and 42 deletions

View File

@ -4793,14 +4793,12 @@ Usable as: Any Matcher
</pre></td></tr>
<tr><td>Matcher&lt;*&gt;</td><td class="name" onclick="toggle('optionally0')"><a name="optionally0Anchor">optionally</a></td><td>Matcher&lt;*&gt;, ..., Matcher&lt;*&gt;</td></tr>
<tr><td colspan="4" class="doc" id="optionally0"><pre>Matches any node regardless of the submatchers.
<tr><td>Matcher&lt;*&gt;</td><td class="name" onclick="toggle('optionally0')"><a name="optionally0Anchor">optionally</a></td><td>Matcher&lt;*&gt;</td></tr>
<tr><td colspan="4" class="doc" id="optionally0"><pre>Matches any node regardless of the submatcher.
However, optionally will generate a result binding for each matching
submatcher.
Useful when additional information which may or may not present about a
main matching node is desired.
However, optionally will retain any bindings generated by the submatcher.
Useful when additional information which may or may not present about a main
matching node is desired.
For example, in:
class Foo {

View File

@ -2586,13 +2586,11 @@ extern const internal::VariadicOperatorMatcherFunc<
2, std::numeric_limits<unsigned>::max()>
allOf;
/// Matches any node regardless of the submatchers.
/// Matches any node regardless of the submatcher.
///
/// However, \c optionally will generate a result binding for each matching
/// submatcher.
///
/// Useful when additional information which may or may not present about a
/// main matching node is desired.
/// However, \c optionally will retain any bindings generated by the submatcher.
/// Useful when additional information which may or may not present about a main
/// matching node is desired.
///
/// For example, in:
/// \code
@ -2612,9 +2610,7 @@ extern const internal::VariadicOperatorMatcherFunc<
/// 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)
///

View File

@ -346,18 +346,11 @@ bool OptionallyVariadicOperator(const DynTypedNode &DynNode,
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<
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 = {};

View File

@ -2007,9 +2007,8 @@ TEST(EachOf, BehavesLikeAnyOfUnlessBothMatch) {
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, SubmatchersDoNotMatchButPreserveBindings) {
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) {