[clang-tidy] support to detect conversion in make_optional for bugprone-optional-value-conversion

[HerrCai0907]

Add support for std::make_optional.

Fixes #119554

[HerrCai0907]

• [clang-tidy] support to detect conversion in make_optional for bugprone-optional-value-conversion #130417 👈 (View in Graphite)
• [AstMatcher]templateArgumentCountIs support FunctionDecl #130416
• [clang-tidy][NFC]refactor matcher for bugprone-optional-value-conversion #130415
• 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

Fixes: #119554

---

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

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp (+18)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
• (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp (+13)

diff --git a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
index 33e823ac07490..a3c6bebe3d17f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -12,6 +12,7 @@
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include <array>
 
 using namespace clang::ast_matchers;
@@ -27,10 +28,15 @@ AST_MATCHER_P(QualType, hasCleanType, Matcher<QualType>, InnerMatcher) {
       Finder, Builder);
 }
 
+AST_MATCHER_P(FunctionDecl, hasReturnType, Matcher<QualType>, InnerMatcher) {
+  return InnerMatcher.matches(Node.getReturnType(), Finder, Builder);
+}
+
 constexpr std::array<StringRef, 2> MakeSmartPtrList{
     "::std::make_unique",
     "::std::make_shared",
 };
+constexpr StringRef MakeOptional = "::std::make_optional";
 
 } // namespace
 
@@ -86,6 +92,18 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
                    callee(functionDecl(
                        matchers::matchesAnyListedName(MakeSmartPtrList),
                        hasTemplateArgument(0, refersToType(BindOptionalType)))),
+                   hasArgument(0, OptionalDerefMatcher)),
+               callExpr(
+                   // match first std::make_optional by limit argument count (1)
+                   // and template count (1).
+                   // 1. template< class T > constexpr
+                   //    std::optional<decay_t<T>> make_optional(T&& value);
+                   // 2. template< class T, class... Args > constexpr
+                   //    std::optional<T> make_optional(Args&&... args);
+                   argumentCountIs(1),
+                   callee(functionDecl(templateArgumentCountIs(1),
+                                       hasName(MakeOptional),
+                                       hasReturnType(BindOptionalType))),
                    hasArgument(0, OptionalDerefMatcher))),
            unless(anyOf(hasAncestor(typeLoc()),
                         hasAncestor(expr(matchers::hasUnevaluatedContext())))))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ce1418a2a7d58..a726015a708f7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,10 @@ Changes in existing checks
   no longer be needed and will be removed. Also fixing false positive from 
   const reference accessors to objects containing optional member.
 
+- Improved :doc:`bugprone-optional-value-conversion
+  <clang-tidy/checks/bugprone/optional-value-conversion>` check to detect
+  conversion in argument of ``std::make_optional``.
+
 - Improved :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
   additional C++ member functions to match.
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
index 768ab1ce014ce..305fd6890710d 100644
--- 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
@@ -27,9 +27,19 @@ class unique_ptr {};
 template <typename type>
 class shared_ptr {};
 
+template <typename T>
+class initializer_list {};
+
 template <class T, class... Args> unique_ptr<T> make_unique(Args &&...args);
 template <class T, class... Args> shared_ptr<T> make_shared(Args &&...args);
 
+template <class T>
+constexpr std::optional<__decay(T)> make_optional(T &&value);
+template <class T, class... Args>
+constexpr std::optional<T> make_optional(Args &&...args);
+template <class T, class U, class... Args>
+constexpr std::optional<T> make_optional(std::initializer_list<U> il, Args &&...args);
+
 } // namespace std
 
 struct A {
@@ -45,9 +55,12 @@ void invalid() {
   // 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]
+  std::make_optional(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());
+  std::make_optional<int>(opt.value());
 }

[PiotrZSL]

Except 2 nits, looks fine.

[HerrCai0907]

Merge activity

• Mar 11, 6:00 PM EDT: A user started a stack merge that includes this pull request via Graphite.
• Mar 11, 6:03 PM EDT: Graphite couldn't merge this pull request because a downstack PR [AstMatcher]templateArgumentCountIs support FunctionDecl #130416 failed to merge.
• Mar 11, 6:10 PM EDT: A user started a stack merge that includes this pull request via Graphite.
• Mar 11, 6:12 PM EDT: A user merged this pull request with Graphite.