[clang-tools-extra] ba373a2 - [clang-tidy]detecting conversion directly by `make_unique` and `make_shared` in bugprone-optional-value-conversion (#119371)

[via cfe-commits]

Author: Congcong Cai
Date: 2024-12-12T06:55:29+08:00
New Revision: ba373a222fe6f65c45a05e9e1114c92580953b79

URL: https://github.com/llvm/llvm-project/commit/ba373a222fe6f65c45a05e9e1114c92580953b79
DIFF: https://github.com/llvm/llvm-project/commit/ba373a222fe6f65c45a05e9e1114c92580953b79.diff

LOG: [clang-tidy]detecting conversion directly by `make_unique` and `make_shared` in bugprone-optional-value-conversion (#119371)

Inspired by #110964

Added: 
    clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp

Modified: 
    clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
index f2ff27d85fb004..55ca4809f058ab 100644
--- a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -12,20 +12,43 @@
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <array>
 
 using namespace clang::ast_matchers;
+using clang::ast_matchers::internal::Matcher;
 
 namespace clang::tidy::bugprone {
 
 namespace {
 
-AST_MATCHER_P(QualType, hasCleanType, ast_matchers::internal::Matcher<QualType>,
-              InnerMatcher) {
+AST_MATCHER_P(QualType, hasCleanType, Matcher<QualType>, InnerMatcher) {
   return InnerMatcher.matches(
       Node.getNonReferenceType().getUnqualifiedType().getCanonicalType(),
       Finder, Builder);
 }
 
+constexpr std::array<StringRef, 2> NameList{
+    "::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(
@@ -67,12 +90,9 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
       callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
                hasArgument(0, ignoringImpCasts(OptionalDereferenceMatcher)));
   Finder->addMatcher(
-      cxxConstructExpr(
-          argumentCountIs(1U), hasType(BindOptionalType),
-          hasArgument(0U, ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
-                                                 StdMoveCallMatcher))),
-          unless(anyOf(hasAncestor(typeLoc()),
-                       hasAncestor(expr(matchers::hasUnevaluatedContext())))))
+      expr(constructFrom(BindOptionalType,
+                         ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
+                                                StdMoveCallMatcher))))
           .bind("expr"),
       this);
 }

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8603c1d8bf1167..6803842106791b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/forwarding-reference-overload>` check by fixing
   a crash when determining if an ``enable_if[_t]`` was found.
 
+- Improved :doc:`bugprone-optional-value-conversion
+  <clang-tidy/checks/bugprone/optional-value-conversion>` to support detecting
+  conversion directly by ``std::make_unique`` and ``std::make_shared``.
+
 - Improved :doc:`bugprone-posix-return
   <clang-tidy/checks/bugprone/posix-return>` check to support integer literals
   as LHS and posix call as RHS of comparison.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp
new file mode 100644
index 00000000000000..768ab1ce014cec
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t
+
+namespace std {
+template <typename T> struct optional {
+  constexpr optional() noexcept;
+  constexpr optional(T &&) noexcept;
+  constexpr optional(const T &) noexcept;
+  template <typename U> constexpr optional(U &&) noexcept;
+  const T &operator*() const;
+  T *operator->();
+  const T *operator->() const;
+  T &operator*();
+  const T &value() const;
+  T &value();
+  const T &get() const;
+  T &get();
+  T value_or(T) const;
+};
+
+template <class T> T &&move(T &x) { return static_cast<T &&>(x); }
+
+template <typename T> class default_delete {};
+
+template <typename type, typename Deleter = std::default_delete<type>>
+class unique_ptr {};
+
+template <typename type>
+class shared_ptr {};
+
+template <class T, class... Args> unique_ptr<T> make_unique(Args &&...args);
+template <class T, class... Args> shared_ptr<T> make_shared(Args &&...args);
+
+} // namespace std
+
+struct A {
+    explicit A (int);
+};
+std::optional<int> opt;
+
+void invalid() {
+  std::make_unique<std::optional<int>>(opt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  using A = std::optional<int>;
+  std::make_unique<A>(opt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  std::make_shared<std::optional<int>>(opt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+}
+
+void valid() {
+  std::make_unique<A>(opt.value());
+  std::make_shared<A>(opt.value());
+}