Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ add_clang_library(clangTidyModernizeModule STATIC
UseTransparentFunctorsCheck.cpp
UseUncaughtExceptionsCheck.cpp
UseUsingCheck.cpp
MakeFunctionToDirectCheck.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please format alphabetically


LINK_LIBS
clangTidy
Expand Down
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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please elaborate why do you need this unless(hasParen(...)) in your check?
I didn't find tests that will use this behavior, or I couldn't understand what existing test used it:(

.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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add newline

39 changes: 39 additions & 0 deletions clang-tools-extra/clang-tidy/modernize/MakeFunctionToDirectCheck.h
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add newline

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Up @@ -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"
Expand Down Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please format alphabetically

}
};

Expand Down
6 changes: 6 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please format alphabetically and place doc string same as other checks. <> should be on the next line


Converts std::make_* function calls to direct constructor calls using CTAD.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sync with docs first sentence (should be identical)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Converts std::make_* function calls to direct constructor calls using CTAD.
Converts ``std::make_*`` function calls to direct constructor calls using CTAD.

Transforms make_optional, make_pair, and make_tuple into equivalent
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Expand Down
46 changes: 46 additions & 0 deletions clang-tools-extra/docs/clang-tidy/checks/modernize/make-direct.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
.. title:: clang-tidy - modernize-make-direct

modernize-make-direct
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add

const int arg = 2.0;
auto t2 = std::make_tuple(1, std::cref(arg), "hi");

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");
}