[clang-tidy][NFC]refactor matcher for bugprone-optional-value-conversion

[HerrCai0907]

The old constructFrom has hidden requirement which TypeMatcher must be used before ArgumentMatcher because there are bind inside.
Inlining this function to make it more intuitive.

[HerrCai0907]

• [clang-tidy] support to detect conversion in make_optional for bugprone-optional-value-conversion #130417
• [AstMatcher]templateArgumentCountIs support FunctionDecl #130416
• [clang-tidy][NFC]refactor matcher for bugprone-optional-value-conversion #130415 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

The old constructFrom has hidden requirement which TypeMatcher must be used before ArgumentMatcher because there are bind inside.
Inlining this function to make it more intuitive.

---

Full diff: https://github.com/llvm/llvm-project/pull/130415.diff

1 Files Affected:

• (modified) clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp (+19-23)

diff --git a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
index 55ca4809f058a..33e823ac07490 100644
--- a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -27,28 +27,11 @@ AST_MATCHER_P(QualType, hasCleanType, Matcher<QualType>, InnerMatcher) {
       Finder, Builder);
 }
 
-constexpr std::array<StringRef, 2> NameList{
+constexpr std::array<StringRef, 2> MakeSmartPtrList{
     "::std::make_unique",
     "::std::make_shared",
 };
 
-Matcher<Expr> constructFrom(Matcher<QualType> TypeMatcher,
-                            Matcher<Expr> ArgumentMatcher) {
-  return expr(
-      anyOf(
-          // construct optional
-          cxxConstructExpr(argumentCountIs(1U), hasType(TypeMatcher),
-                           hasArgument(0U, ArgumentMatcher)),
-          // known template methods in std
-          callExpr(argumentCountIs(1),
-                   callee(functionDecl(
-                       matchers::matchesAnyListedName(NameList),
-                       hasTemplateArgument(0, refersToType(TypeMatcher)))),
-                   hasArgument(0, ArgumentMatcher))),
-      unless(anyOf(hasAncestor(typeLoc()),
-                   hasAncestor(expr(matchers::hasUnevaluatedContext())))));
-}
-
 } // namespace
 
 OptionalValueConversionCheck::OptionalValueConversionCheck(
@@ -74,7 +57,7 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
   auto EqualsBoundOptionalType =
       qualType(hasCleanType(equalsBoundNode("optional-type")));
 
-  auto OptionalDereferenceMatcher = callExpr(
+  auto OptionalDerefMatcherImpl = callExpr(
       anyOf(
           cxxOperatorCallExpr(hasOverloadedOperatorName("*"),
                               hasUnaryOperand(hasType(EqualsBoundOptionalType)))
@@ -88,11 +71,24 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
 
   auto StdMoveCallMatcher =
       callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
-               hasArgument(0, ignoringImpCasts(OptionalDereferenceMatcher)));
+               hasArgument(0, ignoringImpCasts(OptionalDerefMatcherImpl)));
+  auto OptionalDerefMatcher =
+      ignoringImpCasts(anyOf(OptionalDerefMatcherImpl, StdMoveCallMatcher));
+
   Finder->addMatcher(
-      expr(constructFrom(BindOptionalType,
-                         ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
-                                                StdMoveCallMatcher))))
+      expr(anyOf(
+               // construct optional
+               cxxConstructExpr(argumentCountIs(1), hasType(BindOptionalType),
+                                hasArgument(0, OptionalDerefMatcher)),
+               // known template methods in std
+               callExpr(
+                   argumentCountIs(1),
+                   callee(functionDecl(
+                       matchers::matchesAnyListedName(MakeSmartPtrList),
+                       hasTemplateArgument(0, refersToType(BindOptionalType)))),
+                   hasArgument(0, OptionalDerefMatcher))),
+           unless(anyOf(hasAncestor(typeLoc()),
+                        hasAncestor(expr(matchers::hasUnevaluatedContext())))))
           .bind("expr"),
       this);
 }

[PiotrZSL]

LGTM

[HerrCai0907]

Merge activity

• Mar 11, 2:50 AM EDT: A user started a stack merge that includes this pull request via Graphite.
• Mar 11, 2:52 AM EDT: A user merged this pull request with Graphite.