-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[clang-tidy] Add modernize-make-direct check #118120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c741fff
ccfe7ff
837b597
cbf72d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
//===--- MakeFunctionToDirectCheck.cpp - clang-tidy ----------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "MakeFunctionToDirectCheck.h" | ||
#include "../utils/TransformerClangTidyCheck.h" | ||
#include "clang/ASTMatchers/ASTMatchers.h" | ||
#include "clang/Tooling/Transformer/RangeSelector.h" | ||
#include "clang/Tooling/Transformer/RewriteRule.h" | ||
#include "clang/Tooling/Transformer/Stencil.h" | ||
|
||
using namespace ::clang::ast_matchers; | ||
using namespace ::clang::transformer; | ||
|
||
namespace clang::tidy::modernize { | ||
|
||
namespace { | ||
|
||
RewriteRuleWith<std::string> | ||
makeFunctionToDirectCheckImpl(bool CheckMakePair, bool CheckMakeOptional, | ||
bool CheckMakeTuple) { | ||
std::vector<RewriteRuleWith<std::string>> Rules; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider using llvm::SmallVector |
||
|
||
// Helper to create a rule for a specific make_* function | ||
auto createRule = [](StringRef MakeFunction, StringRef DirectType) { | ||
auto WarningMessage = cat("use class template argument deduction (CTAD) " | ||
"instead of ", | ||
MakeFunction); | ||
|
||
return makeRule( | ||
callExpr(callee(functionDecl(hasName(MakeFunction))), | ||
unless(hasParent(implicitCastExpr(hasImplicitDestinationType( | ||
qualType(hasCanonicalType(qualType(asString("void"))))))))) | ||
Comment on lines
+36
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please elaborate why do you need this |
||
.bind("make_call"), | ||
changeTo(node("make_call"), | ||
cat(DirectType, "(", callArgs("make_call"), ")")), | ||
WarningMessage); | ||
}; | ||
|
||
if (CheckMakeOptional) { | ||
Rules.push_back(createRule("std::make_optional", "std::optional")); | ||
} | ||
|
||
if (CheckMakePair) { | ||
Rules.push_back(createRule("std::make_pair", "std::pair")); | ||
} | ||
|
||
if (CheckMakeTuple) { | ||
Rules.push_back(createRule("std::make_tuple", "std::tuple")); | ||
} | ||
|
||
return applyFirst(Rules); | ||
} | ||
|
||
} // namespace | ||
|
||
MakeFunctionToDirectCheck::MakeFunctionToDirectCheck(StringRef Name, | ||
ClangTidyContext *Context) | ||
: utils::TransformerClangTidyCheck(Name, Context), | ||
CheckMakePair(Options.get("CheckMakePair", true)), | ||
CheckMakeOptional(Options.get("CheckMakeOptional", true)), | ||
CheckMakeTuple(Options.get("CheckMakeTuple", true)) { | ||
setRule(makeFunctionToDirectCheckImpl(CheckMakePair, CheckMakeOptional, | ||
CheckMakeTuple)); | ||
} | ||
|
||
void MakeFunctionToDirectCheck::storeOptions( | ||
ClangTidyOptions::OptionMap &Opts) { | ||
Options.store(Opts, "CheckMakePair", CheckMakePair); | ||
Options.store(Opts, "CheckMakeOptional", CheckMakeOptional); | ||
Options.store(Opts, "CheckMakeTuple", CheckMakeTuple); | ||
} | ||
|
||
} // namespace clang::tidy::modernize | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
//===--- MakeFunctionToDirectCheck.h - clang-tidy --------------*- C++ -*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKEFUNCTIONTODIRECTCHECK_H | ||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKEFUNCTIONTODIRECTCHECK_H | ||
|
||
#include "../utils/TransformerClangTidyCheck.h" | ||
|
||
namespace clang::tidy::modernize { | ||
|
||
/// Converts std::make_* function calls to direct constructor calls using | ||
/// class template argument deduction (CTAD). | ||
/// | ||
/// For the user-facing documentation see: | ||
/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/make-direct.html | ||
class MakeFunctionToDirectCheck : public utils::TransformerClangTidyCheck { | ||
public: | ||
MakeFunctionToDirectCheck(StringRef Name, ClangTidyContext *Context); | ||
|
||
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { | ||
return LangOpts.CPlusPlus17; | ||
} | ||
|
||
void storeOptions(ClangTidyOptions::OptionMap &Opts) override; | ||
|
||
private: | ||
const bool CheckMakePair; | ||
const bool CheckMakeOptional; | ||
const bool CheckMakeTuple; | ||
}; | ||
|
||
} // namespace clang::tidy::modernize | ||
|
||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKEFUNCTIONTODIRECTCHECK_H | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add newline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add newline. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
#include "DeprecatedIosBaseAliasesCheck.h" | ||
#include "LoopConvertCheck.h" | ||
#include "MacroToEnumCheck.h" | ||
#include "MakeFunctionToDirectCheck.h" | ||
#include "MakeSharedCheck.h" | ||
#include "MakeUniqueCheck.h" | ||
#include "MinMaxUseInitializerListCheck.h" | ||
|
@@ -125,6 +126,8 @@ class ModernizeModule : public ClangTidyModule { | |
CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( | ||
"modernize-use-uncaught-exceptions"); | ||
CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); | ||
CheckFactories.registerCheck<MakeFunctionToDirectCheck>( | ||
"modernize-make-direct"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please format alphabetically |
||
} | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -124,6 +124,12 @@ New checks | |||||
pointer and store it as class members without handle the copy and move | ||||||
constructors and the assignments. | ||||||
|
||||||
- New :doc:`modernize-make-direct <clang-tidy/checks/modernize/make-direct>` check. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please format alphabetically and place doc string same as other checks. |
||||||
|
||||||
Converts std::make_* function calls to direct constructor calls using CTAD. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please sync with docs first sentence (should be identical) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
Transforms make_optional, make_pair, and make_tuple into equivalent | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Single sentence is enough. |
||||||
direct constructor calls using C++17's class template argument deduction. | ||||||
|
||||||
- New :doc:`bugprone-misleading-setter-of-reference | ||||||
<clang-tidy/checks/bugprone/misleading-setter-of-reference>` check. | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
.. title:: clang-tidy - modernize-make-direct | ||
|
||
modernize-make-direct | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sync check name accross all files, somewhere you have "MakeFunctionToDirect". |
||
===================== | ||
|
||
Replaces ``std::make_*`` function calls with direct constructor calls using class template | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please synchronize with statement in Release Notes. |
||
argument deduction (CTAD). | ||
|
||
================================== ==================================== | ||
Before After | ||
---------------------------------- ------------------------------------ | ||
``std::make_optional<int>(42)`` ``std::optional(42)`` | ||
``std::make_pair(1, "test")`` ``std::pair(1, "test")`` | ||
``std::make_tuple(1, 2.0, "hi")`` ``std::tuple(1, 2.0, "hi")`` | ||
================================== ==================================== | ||
|
||
.. note:: | ||
|
||
This check does not transform ``std::make_unique`` or ``std::make_shared`` because: | ||
|
||
1. These smart pointer types cannot be constructed using CTAD from raw pointers. | ||
2. ``std::make_shared`` provides performance benefits (single allocation) and | ||
exception safety that would be lost with direct construction. | ||
3. Direct use of ``new`` is discouraged in modern C++ code. | ||
|
||
Use the dedicated ``modernize-make-unique`` and ``modernize-make-shared`` checks | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is confusing, since these checks do the exact opposite of what this check is doing. I would vote for removing this line. |
||
for transforming these functions. | ||
|
||
Options | ||
------- | ||
|
||
.. option:: CheckMakePair | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should instead be 1 option with a semicolon separated list of which functions to transform. |
||
|
||
When `true`, transforms ``std::make_pair`` calls to direct constructor calls. | ||
Default is `true`. | ||
|
||
.. option:: CheckMakeOptional | ||
|
||
When `true`, transforms ``std::make_optional`` calls to direct constructor calls. | ||
Default is `true`. | ||
|
||
.. option:: CheckMakeTuple | ||
|
||
When `true`, transforms ``std::make_tuple`` calls to direct constructor calls. | ||
Default is `true`. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
// RUN: %check_clang_tidy -std=c++17 %s modernize-make-direct %t | ||
|
||
namespace std { | ||
template<typename T> | ||
struct optional { | ||
optional(const T&) {} | ||
}; | ||
|
||
template<typename T> | ||
optional<T> make_optional(const T& t) { return optional<T>(t); } | ||
|
||
template<typename T> | ||
struct unique_ptr { | ||
explicit unique_ptr(T*) {} | ||
}; | ||
|
||
template<typename T, typename... Args> | ||
unique_ptr<T> make_unique(Args&&... args) { | ||
return unique_ptr<T>(new T(args...)); | ||
} | ||
|
||
template<typename T> | ||
struct shared_ptr { | ||
shared_ptr(T*) {} | ||
}; | ||
|
||
template<typename T, typename... Args> | ||
shared_ptr<T> make_shared(Args&&... args) { | ||
return shared_ptr<T>(new T(args...)); | ||
} | ||
|
||
template<typename T, typename U> | ||
struct pair { | ||
T first; | ||
U second; | ||
pair(const T& x, const U& y) : first(x), second(y) {} | ||
}; | ||
|
||
template<typename T, typename U> | ||
pair<T,U> make_pair(T&& t, U&& u) { | ||
return pair<T,U>(t, u); | ||
} | ||
|
||
template<typename... T> | ||
struct tuple { | ||
tuple(const T&... args) {} | ||
}; | ||
|
||
template<typename... T> | ||
tuple<T...> make_tuple(T&&... t) { | ||
return tuple<T...>(t...); | ||
} | ||
} | ||
|
||
struct Widget { | ||
Widget(int x) {} | ||
}; | ||
|
||
|
||
void basic_tests() { | ||
auto o1 = std::make_optional<int>(42); | ||
// CHECK-MESSAGES: warning: use class template argument deduction (CTAD) instead of std::make_optional [modernize-make-direct] | ||
// CHECK-FIXES: auto o1 = std::optional(42); | ||
|
||
// make_unique and make_shared are not transformed by this check | ||
auto u1 = std::make_unique<Widget>(1); | ||
auto s1 = std::make_shared<Widget>(2); | ||
|
||
auto p1 = std::make_pair(1, "test"); | ||
// CHECK-MESSAGES: warning: use class template argument deduction (CTAD) instead of std::make_pair [modernize-make-direct] | ||
// CHECK-FIXES: auto p1 = std::pair(1, "test"); | ||
|
||
auto t1 = std::make_tuple(1, 2.0, "hi"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add
this check must be silent in this case |
||
// CHECK-MESSAGES: warning: use class template argument deduction (CTAD) instead of std::make_tuple [modernize-make-direct] | ||
// CHECK-FIXES: auto t1 = std::tuple(1, 2.0, "hi"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please format alphabetically