llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> Fixes https://github.com/llvm/llvm-project/issues/61454. --- Patch is 40.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/184009.diff 13 Files Affected: - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp (+3) - (modified) clang-tools-extra/clang-tidy/misc/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp (+3) - (added) clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.cpp (+160) - (added) clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.h (+34) - (modified) clang-tools-extra/clang-tidy/utils/LexerUtils.h (+15) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+11) - (added) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-braced-initialization.rst (+15) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+2) - (added) clang-tools-extra/docs/clang-tidy/checks/misc/use-braced-initialization.rst (+66) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string (+7-3) - (added) clang-tools-extra/test/clang-tidy/checkers/misc/use-braced-initialization-cxx20.cpp (+211) - (added) clang-tools-extra/test/clang-tidy/checkers/misc/use-braced-initialization.cpp (+603) ``````````diff diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index fab4f92be22b6..e60e43c5e5697 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -11,6 +11,7 @@ #include "../bugprone/NarrowingConversionsCheck.h" #include "../misc/NonPrivateMemberVariablesInClassesCheck.h" #include "../misc/UnconventionalAssignOperatorCheck.h" +#include "../misc/UseBracedInitializationCheck.h" #include "../modernize/AvoidCArraysCheck.h" #include "../modernize/MacroToEnumCheck.h" #include "../modernize/UseDefaultMemberInitCheck.h" @@ -133,6 +134,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { CheckFactories.registerCheck<SpecialMemberFunctionsCheck>( "cppcoreguidelines-special-member-functions"); CheckFactories.registerCheck<SlicingCheck>("cppcoreguidelines-slicing"); + CheckFactories.registerCheck<misc::UseBracedInitializationCheck>( + "cppcoreguidelines-use-braced-initialization"); CheckFactories.registerCheck<modernize::UseDefaultMemberInitCheck>( "cppcoreguidelines-use-default-member-init"); CheckFactories.registerCheck<UseEnumClassCheck>( diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index e34b0cf687be3..215243bae96aa 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -45,6 +45,7 @@ add_clang_library(clangTidyMiscModule STATIC UnusedParametersCheck.cpp UnusedUsingDeclsCheck.cpp UseAnonymousNamespaceCheck.cpp + UseBracedInitializationCheck.cpp UseInternalLinkageCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index f8550b30b9789..3abd74ff94f31 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -34,6 +34,7 @@ #include "UnusedParametersCheck.h" #include "UnusedUsingDeclsCheck.h" #include "UseAnonymousNamespaceCheck.h" +#include "UseBracedInitializationCheck.h" #include "UseInternalLinkageCheck.h" namespace clang::tidy { @@ -90,6 +91,8 @@ class MiscModule : public ClangTidyModule { "misc-unused-using-decls"); CheckFactories.registerCheck<UseAnonymousNamespaceCheck>( "misc-use-anonymous-namespace"); + CheckFactories.registerCheck<UseBracedInitializationCheck>( + "misc-use-braced-initialization"); CheckFactories.registerCheck<UseInternalLinkageCheck>( "misc-use-internal-linkage"); } diff --git a/clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.cpp new file mode 100644 index 0000000000000..7a9465e12581f --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.cpp @@ -0,0 +1,160 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UseBracedInitializationCheck.h" +#include "../utils/LexerUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::misc { + +namespace { + +AST_MATCHER_P(VarDecl, hasInitStyle, VarDecl::InitializationStyle, Style) { + return Node.getInitStyle() == Style; +} + +AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); } + +AST_MATCHER(CXXConstructExpr, noMacroParens) { + const SourceRange Range = Node.getParenOrBraceRange(); + return Range.isValid() && !Range.getBegin().isMacroID() && + !Range.getEnd().isMacroID(); +} + +AST_MATCHER(Expr, isCXXParenListInitExpr) { + return isa<CXXParenListInitExpr>(Node); +} + +/// Matches CXXConstructExpr whose target class has any constructor +/// taking 'std::initializer_list'. When such a constructor exists, braced +/// initialization may call it instead of the intended constructor. +AST_MATCHER(CXXConstructExpr, constructsTypeWithInitListCtor) { + const CXXRecordDecl *RD = Node.getConstructor()->getParent(); + if (!RD || !RD->hasDefinition()) + return false; + return llvm::any_of(RD->ctors(), [](const CXXConstructorDecl *Ctor) { + if (Ctor->getNumParams() == 0) + return false; + const QualType FirstParam = + Ctor->getParamDecl(0)->getType().getNonReferenceType(); + const auto *Record = FirstParam->getAsCXXRecordDecl(); + if (!Record || !Record->getDeclName().isIdentifier() || + Record->getName() != "initializer_list" || !Record->isInStdNamespace()) + return false; + // [dcl.init.list] p2: all other params must have defaults. + for (unsigned I = 1; I < Ctor->getNumParams(); ++I) + if (!Ctor->getParamDecl(I)->hasDefaultArg()) + return false; + return true; + }); +} + +} // namespace + +void UseBracedInitializationCheck::registerMatchers(MatchFinder *Finder) { + auto GoodCtor = allOf( + noMacroParens(), unless(constructsTypeWithInitListCtor()), + unless(isInTemplateInstantiation()), unless(isListInitialization())); + auto GoodCtorExpr = cxxConstructExpr(GoodCtor).bind("ctor"); + auto GoodVar = + allOf(unless(hasType(isDependentType())), unless(hasType(autoType()))); + + // Variable declarations: Simple w(1), Takes t({1, 2}) + Finder->addMatcher(varDecl(hasInitStyle(VarDecl::CallInit), + hasInitializer(ignoringImplicit(GoodCtorExpr)), + GoodVar), + this); + + // Scalar direct-init: int x(42), double d(3.14) + Finder->addMatcher( + varDecl(hasInitStyle(VarDecl::CallInit), + hasInitializer(unless(ignoringImplicit(cxxConstructExpr()))), + GoodVar) + .bind("scalar"), + this); + + Finder->addMatcher(cxxFunctionalCastExpr(has(GoodCtorExpr)), this); + Finder->addMatcher(cxxTemporaryObjectExpr(GoodCtor).bind("ctor"), this); + Finder->addMatcher(cxxNewExpr(has(GoodCtorExpr)), this); + + if (getLangOpts().CPlusPlus20) { + auto GoodPLE = expr(isCXXParenListInitExpr()).bind("ple"); + Finder->addMatcher(varDecl(hasInitStyle(VarDecl::ParenListInit), + hasInitializer(GoodPLE), GoodVar) + .bind("var_ple"), + this); + Finder->addMatcher(cxxFunctionalCastExpr(has(GoodPLE)).bind("cast_ple"), + this); + Finder->addMatcher(cxxNewExpr(has(GoodPLE)).bind("new_ple"), this); + } +} + +void UseBracedInitializationCheck::check( + const MatchFinder::MatchResult &Result) { + SourceLocation DiagLoc; + SourceLocation LParen; + SourceLocation RParen; + + if (const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor")) { + DiagLoc = Ctor->getBeginLoc(); + LParen = Ctor->getParenOrBraceRange().getBegin(); + RParen = Ctor->getParenOrBraceRange().getEnd(); + } else if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("scalar")) { + assert(Var->hasInit()); + const SourceManager &SM = *Result.SourceManager; + const LangOptions &LangOpts = Result.Context->getLangOpts(); + + const std::optional<Token> LTok = + utils::lexer::findPreviousTokenSkippingComments( + Var->getInit()->getBeginLoc(), SM, LangOpts); + if (!LTok || LTok->isNot(tok::l_paren) || LTok->getLocation().isMacroID()) + return; + + const std::optional<Token> RTok = + utils::lexer::findNextTokenSkippingComments(Var->getInit()->getEndLoc(), + SM, LangOpts); + if (!RTok || RTok->isNot(tok::r_paren) || RTok->getLocation().isMacroID()) + return; + + DiagLoc = Var->getLocation(); + LParen = LTok->getLocation(); + RParen = RTok->getLocation(); + } else if (const auto *PLE = + Result.Nodes.getNodeAs<CXXParenListInitExpr>("ple")) { + LParen = PLE->getBeginLoc(); + RParen = PLE->getEndLoc(); + if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var_ple")) + DiagLoc = Var->getLocation(); + else if (const auto *Cast = + Result.Nodes.getNodeAs<CXXFunctionalCastExpr>("cast_ple")) + DiagLoc = Cast->getBeginLoc(); + else if (const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>("new_ple")) + DiagLoc = New->getBeginLoc(); + else + llvm_unreachable("No context for CXXParenListInitExpr"); + } else { + llvm_unreachable("No matches found"); + } + + if (LParen.isMacroID() || RParen.isMacroID()) + return; + + diag(DiagLoc, + "use braced initialization instead of parenthesized initialization") + << FixItHint::CreateReplacement(LParen, "{") + << FixItHint::CreateReplacement(RParen, "}"); +} + +} // namespace clang::tidy::misc diff --git a/clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.h b/clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.h new file mode 100644 index 0000000000000..a010558ab1d2a --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/UseBracedInitializationCheck.h @@ -0,0 +1,34 @@ +//===----------------------------------------------------------------------===// +// +// 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_MISC_USEBRACEDINITIALIZATIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEBRACEDINITIALIZATIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::misc { + +/// Suggests replacing parenthesized initialization with braced +/// initialization. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/misc/use-braced-initialization.html +class UseBracedInitializationCheck : public ClangTidyCheck { +public: + UseBracedInitializationCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } +}; + +} // namespace clang::tidy::misc + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEBRACEDINITIALIZATIONCHECK_H diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h index 38123ae14cff7..259012a7b528b 100644 --- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h @@ -93,6 +93,21 @@ SourceLocation findNextAnyTokenKind(SourceLocation Start, } } +// Finds previous token, possibly a comment. +inline std::optional<Token> +findPreviousTokenIncludingComments(SourceLocation Start, + const SourceManager &SM, + const LangOptions &LangOpts) { + return Lexer::findPreviousToken(Start, SM, LangOpts, true); +} + +// Finds previous token that's not a comment. +inline std::optional<Token> +findPreviousTokenSkippingComments(SourceLocation Start, const SourceManager &SM, + const LangOptions &LangOpts) { + return Lexer::findPreviousToken(Start, SM, LangOpts, false); +} + // Finds next token, possibly a comment. inline std::optional<Token> findNextTokenIncludingComments(SourceLocation Start, const SourceManager &SM, diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6bdc0ae7bdcc8..8a9d03127a890 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -124,6 +124,12 @@ New checks ``llvm::to_vector(llvm::make_filter_range(...))`` that can be replaced with ``llvm::map_to_vector`` and ``llvm::filter_to_vector``. +- New :doc:`misc-use-braced-initialization + <clang-tidy/checks/misc/use-braced-initialization>` check. + + Suggests replacing parenthesized initialization with braced + initialization. + - New :doc:`modernize-use-string-view <clang-tidy/checks/modernize/use-string-view>` check. @@ -151,6 +157,11 @@ New checks New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cppcoreguidelines-use-braced-initialization + <clang-tidy/checks/cppcoreguidelines/use-braced-initialization>` to + :doc:`misc-use-braced-initialization + <clang-tidy/checks/misc/use-braced-initialization>` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-braced-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-braced-initialization.rst new file mode 100644 index 0000000000000..3deb40f96f1cc --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-braced-initialization.rst @@ -0,0 +1,15 @@ +.. title:: clang-tidy - cppcoreguidelines-use-braced-initialization +.. meta:: + :http-equiv=refresh: 5;URL=../misc/use-braced-initialization.html + +cppcoreguidelines-use-braced-initialization +=========================================== + +This check implements `ES.23 +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax>`_ +from the C++ Core Guidelines. + +The `cppcoreguidelines-use-braced-initialization` check is an alias, +please see +:doc:`misc-use-braced-initialization<../misc/use-braced-initialization>` +for more information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c475870ed7b31..66dce8f492e80 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -287,6 +287,7 @@ Clang-Tidy Checks :doc:`misc-unused-parameters <misc/unused-parameters>`, "Yes" :doc:`misc-unused-using-decls <misc/unused-using-decls>`, "Yes" :doc:`misc-use-anonymous-namespace <misc/use-anonymous-namespace>`, + :doc:`misc-use-braced-initialization <misc/use-braced-initialization>`, "Yes" :doc:`misc-use-internal-linkage <misc/use-internal-linkage>`, "Yes" :doc:`modernize-avoid-bind <modernize/avoid-bind>`, "Yes" :doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`, @@ -587,6 +588,7 @@ Check aliases :doc:`cppcoreguidelines-noexcept-move-operations <cppcoreguidelines/noexcept-move-operations>`, :doc:`performance-noexcept-move-constructor <performance/noexcept-move-constructor>`, "Yes" :doc:`cppcoreguidelines-noexcept-swap <cppcoreguidelines/noexcept-swap>`, :doc:`performance-noexcept-swap <performance/noexcept-swap>`, "Yes" :doc:`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines/non-private-member-variables-in-classes>`, :doc:`misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes>`, + :doc:`cppcoreguidelines-use-braced-initialization <cppcoreguidelines/use-braced-initialization>`, :doc:`misc-use-braced-initialization <misc/use-braced-initialization>`, "Yes" :doc:`cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init>`, :doc:`modernize-use-default-member-init <modernize/use-default-member-init>`, "Yes" :doc:`fuchsia-header-anon-namespaces <fuchsia/header-anon-namespaces>`, :doc:`misc-anonymous-namespace-in-header <misc/anonymous-namespace-in-header>`, :doc:`fuchsia-multiple-inheritance <fuchsia/multiple-inheritance>`, :doc:`misc-multiple-inheritance <misc/multiple-inheritance>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-braced-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-braced-initialization.rst new file mode 100644 index 0000000000000..4df9480b614b0 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-braced-initialization.rst @@ -0,0 +1,66 @@ +.. title:: clang-tidy - misc-use-braced-initialization + +misc-use-braced-initialization +============================== + +Suggests replacing parenthesized initialization with braced initialization. + +Braced initialization has several advantages over parenthesized initialization: + +- **Catches narrowing conversions.** ``int x{3.14}`` is a compile-time error, + while ``int x(3.14)`` silently truncates. +- **Uniform syntax.** Braces work consistently for aggregates, containers, and + constructors, giving a single initialization style across all types. + +For example: + +.. code-block:: c++ + + struct Matrix { + Matrix(int rows, int cols); + }; + + // Variable declarations: + Matrix m(3, 4); // -> Matrix m{3, 4}; + int n(42); // -> int n{42}; + + // Copy initialization: + Matrix m = Matrix(3, 4); // -> Matrix m = Matrix{3, 4}; + + // Temporary objects: + use(Matrix(3, 4)); // -> use(Matrix{3, 4}); + + // New expressions: + auto *p = new Matrix(3, 4); // -> auto *p = new Matrix{3, 4}; + +The check skips cases where changing from ``()`` to ``{}`` would alter program +semantics: + +- Types that have any constructor accepting ``std::initializer_list``, since + braced initialization would prefer that overload and silently change + semantics. +- Direct-initialized ``auto`` variables, where deduction rules may differ + between C++ standards. +- Expressions in macro expansions. + +.. note:: + + Braced initialization prohibits implicit narrowing conversions. + In some cases the suggested fix may introduce a compiler error. + + .. code-block:: c++ + + struct Foo { + Foo(short n); + }; + + int n = 10; + Foo f(n); // OK: implicit narrowing allowed with () + Foo f{n}; // error: narrowing conversion from 'int' to 'short' + +References +---------- + +This check corresponds to the CERT C++ Core Guidelines rule +`C++ Core Guidelines ES.23 +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax>`_. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string index fc197d0afa714..6ff5afb4d781c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string @@ -4,6 +4,7 @@ // For size_t #include "string.h" #include "memory" +#include "initializer_list" typedef unsigned __INT16_TYPE__ char16; typedef unsigned __INT32_TYPE__ char32; @@ -26,6 +27,7 @@ struct basic_string { basic_string(const C *p, size_type count); basic_string(const C *b, const C *e); basic_string(size_t, C); + basic_string(std::initializer_list<C>); operator basic_string_view<C, T>() const; ~basic_string(); @@ -181,14 +183,16 @@ bool operator!=(const char*, const std::string_view&); size_t strlen(const char* str); -namespace literals { -namespace string_literals { +#if __cplusplus >= 201402L +inline namespace literals { +inline namespace string_literals { string operator""s(const char *, size_t); } -namespace string_view_literals { +inline namespace string_view_literals { string_view operator""sv(const char *, size_t); } } +#endif } #endif // _STRING_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-braced-initialization-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-braced-initialization-cxx20.cpp new file mode 100644 index 0000000000000..9da81b5a7d743 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-braced-initialization-cxx20.cpp @@ -0,0 +1,211 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s misc-use-braced-initialization %t + +struct Agg { + int a, b; +}; + +struct AggDefault { + int a = 0; + int b; +}; + +struct Nested { + Agg x; + int y; +}; + +struct Takes { + Ta... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/184009 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
