llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Grigory Pastukhov (grigorypas)
<details>
<summary>Changes</summary>
`modernize-return-braced-init-list` rewrites `return T(args)` to `return
T{args}`. When `T` has a `std::initializer_list` constructor this can change
behavior: per [over.match.list], list-initialization prefers initializer-list
constructors, so the braced form may call a different constructor than the
parenthesized call.
---
Full diff: https://github.com/llvm/llvm-project/pull/205440.diff
6 Files Affected:
- (modified) clang-tools-extra/clang-tidy/misc/ExplicitConstructorCheck.cpp
(+2-22)
- (modified)
clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp (+30)
- (modified) clang-tools-extra/clang-tidy/utils/TypeTraits.cpp (+22)
- (modified) clang-tools-extra/clang-tidy/utils/TypeTraits.h (+3)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
(+54-1)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/misc/ExplicitConstructorCheck.cpp
b/clang-tools-extra/clang-tidy/misc/ExplicitConstructorCheck.cpp
index 8c6f8ef978991..23261fe5af61b 100644
--- a/clang-tools-extra/clang-tidy/misc/ExplicitConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/ExplicitConstructorCheck.cpp
@@ -8,6 +8,7 @@
#include "ExplicitConstructorCheck.h"
#include "../utils/LexerUtils.h"
+#include "../utils/TypeTraits.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -31,27 +32,6 @@ void ExplicitConstructorCheck::registerMatchers(MatchFinder
*Finder) {
this);
}
-static bool declIsStdInitializerList(const NamedDecl *D) {
- // First use the fast getName() method to avoid unnecessary calls to the
- // slow getQualifiedNameAsString().
- return D->getName() == "initializer_list" &&
- D->getQualifiedNameAsString() == "std::initializer_list";
-}
-
-static bool isStdInitializerList(QualType Type) {
- Type = Type.getCanonicalType();
- if (const auto *TS = Type->getAs<TemplateSpecializationType>()) {
- if (const TemplateDecl *TD = TS->getTemplateName().getAsTemplateDecl())
- return declIsStdInitializerList(TD);
- }
- if (const auto *RT = Type->getAs<RecordType>()) {
- if (const auto *Specialization =
- dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl()))
- return
declIsStdInitializerList(Specialization->getSpecializedTemplate());
- }
- return false;
-}
-
void ExplicitConstructorCheck::check(const MatchFinder::MatchResult &Result) {
constexpr char NoExpressionWarningMessage[] =
"%0 must be marked explicit to avoid unintentional implicit conversions";
@@ -79,7 +59,7 @@ void ExplicitConstructorCheck::check(const
MatchFinder::MatchResult &Result) {
const ExplicitSpecifier ExplicitSpec = Ctor->getExplicitSpecifier();
- const bool TakesInitializerList = isStdInitializerList(
+ const bool TakesInitializerList = utils::type_traits::isStdInitializerList(
Ctor->getParamDecl(0)->getType().getNonReferenceType());
if (ExplicitSpec.isExplicit() &&
(Ctor->isCopyOrMoveConstructor() || TakesInitializerList)) {
diff --git
a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
index 65da5b1a2b403..0731707d7126b 100644
--- a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
@@ -7,7 +7,9 @@
//===----------------------------------------------------------------------===//
#include "ReturnBracedInitListCheck.h"
+#include "../utils/TypeTraits.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
@@ -16,6 +18,27 @@ using namespace clang::ast_matchers;
namespace clang::tidy::modernize {
+namespace {
+bool hasInitListConstructor(const CXXRecordDecl *RD) {
+ if (RD == nullptr || !RD->hasDefinition())
+ return false;
+ auto IsInitListCtor = [](const CXXConstructorDecl *Ctor) {
+ return Ctor->hasOneParamOrDefaultArgs() &&
+ utils::type_traits::isStdInitializerList(
+ Ctor->getParamDecl(0)->getType().getNonReferenceType());
+ };
+ return llvm::any_of(RD->decls(), [&](const Decl *D) {
+ if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(D))
+ return IsInitListCtor(Ctor);
+ if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
+ if (const auto *Ctor =
+ dyn_cast<CXXConstructorDecl>(FTD->getTemplatedDecl()))
+ return IsInitListCtor(Ctor);
+ return false;
+ });
+}
+} // namespace
+
void ReturnBracedInitListCheck::registerMatchers(MatchFinder *Finder) {
auto SemanticallyDifferentContainer = allOf(
hasDeclaration(
@@ -66,6 +89,13 @@ void ReturnBracedInitListCheck::check(const
MatchFinder::MatchResult &Result) {
if (ReturnType != ConstructType)
return;
+ // Rewriting `T(args)` to a braced-init-list changes overload resolution when
+ // `T` has a std::initializer_list constructor: list-initialization prefers
+ // the initializer_list overload, so the braced form may silently select a
+ // different constructor than the parenthesized call.
+ if (hasInitListConstructor(ConstructType->getAsCXXRecordDecl()))
+ return;
+
auto Diag = diag(Loc, "avoid repeating the return type from the "
"declaration; use a braced initializer list instead");
diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
index c15d90b19d359..06d489129cd43 100644
--- a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
@@ -9,6 +9,7 @@
#include "TypeTraits.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
#include <optional>
namespace clang::tidy::utils::type_traits {
@@ -146,4 +147,25 @@ bool hasNonTrivialMoveAssignment(QualType Type) {
Record->hasNonTrivialMoveAssignment();
}
+static bool declIsStdInitializerList(const NamedDecl *D) {
+ // First use the fast getName() method to avoid unnecessary calls to the
+ // slow getQualifiedNameAsString().
+ return D->getName() == "initializer_list" &&
+ D->getQualifiedNameAsString() == "std::initializer_list";
+}
+
+bool isStdInitializerList(QualType Type) {
+ Type = Type.getCanonicalType();
+ if (const auto *TS = Type->getAs<TemplateSpecializationType>()) {
+ if (const TemplateDecl *TD = TS->getTemplateName().getAsTemplateDecl())
+ return declIsStdInitializerList(TD);
+ }
+ if (const auto *RT = Type->getAs<RecordType>()) {
+ if (const auto *Specialization =
+ dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl()))
+ return
declIsStdInitializerList(Specialization->getSpecializedTemplate());
+ }
+ return false;
+}
+
} // namespace clang::tidy::utils::type_traits
diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.h
b/clang-tools-extra/clang-tidy/utils/TypeTraits.h
index 98a4a99bf8d4d..03711461b51ac 100644
--- a/clang-tools-extra/clang-tidy/utils/TypeTraits.h
+++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.h
@@ -34,6 +34,9 @@ bool hasNonTrivialMoveConstructor(QualType Type);
/// Return true if `Type` has a non-trivial move assignment operator.
bool hasNonTrivialMoveAssignment(QualType Type);
+/// Returns `true` if `Type` is a `std::initializer_list<...>` specialization.
+bool isStdInitializerList(QualType Type);
+
} // namespace clang::tidy::utils::type_traits
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 8871b37ddb1bf..3f0b61c57fb6b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -627,6 +627,12 @@ Changes in existing checks
<clang-tidy/checks/modernize/return-braced-init-list>` check to apply fix-it
when type qualifiers and/or reference modifiers are used with parameters.
+- Improved :doc:`modernize-return-braced-init-list
+ <clang-tidy/checks/modernize/return-braced-init-list>` check by no longer
+ rewriting the return value when the constructed type has a
+ ``std::initializer_list`` constructor, as the braced form could silently
+ select a different constructor.
+
- Improved :doc:`modernize-use-default-member-init
<clang-tidy/checks/modernize/use-default-member-init>` check by fixing a
false positive when a constructor initializer refers to a declaration that
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
index 5b0e7e7fb97f3..d8e5caa6f3c87 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
@@ -66,8 +66,10 @@ Foo f6() {
std::vector<int> vectorWithOneParameter() {
int i7 = 1;
+ // No warning: std::vector has a std::initializer_list constructor, so
+ // `std::vector<int>{i7}` would select it (a 1-element vector) instead of
+ // `std::vector<int>(i7)` (an i7-element vector). Rewriting is unsound.
return std::vector<int>(i7);
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: avoid repeating the return type
}
std::vector<int> vectorIntWithTwoParameter() {
@@ -84,6 +86,57 @@ std::vector<A> vectorRecordWithTwoParameter() {
}
+struct WithInitList {
+ WithInitList(Bar) {}
+ WithInitList(std::initializer_list<WithInitList>) {}
+};
+
+// No warning: braces would select the std::initializer_list constructor
+// instead of WithInitList(Bar), changing overload resolution.
+WithInitList initListCtorNoRewrite() {
+ Bar b;
+ return WithInitList(b);
+}
+
+WithInitList initListCtorNoRewriteArg(Bar b) {
+ return WithInitList(b);
+}
+
+// `std::initializer_list` as the first parameter but with a non-defaulted
+// trailing parameter is NOT an initializer-list constructor, so braces cannot
+// select it. A warning is expected.
+struct NotInitListCtor {
+ NotInitListCtor(std::initializer_list<int>, int);
+ NotInitListCtor(Bar) {}
+};
+
+NotInitListCtor notInitListCtorRewrite(Bar b) {
+ return NotInitListCtor(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: avoid repeating the return type
+ // CHECK-FIXES: return {b};
+}
+
+struct TemplatedInitListCtor {
+ TemplatedInitListCtor(Bar) {}
+ template <typename T>
+ TemplatedInitListCtor(std::initializer_list<T>) {}
+};
+
+TemplatedInitListCtor templatedInitListCtorNoRewrite(Bar b) {
+ return TemplatedInitListCtor(b);
+}
+
+// An initializer-list constructor may take the list by (cv-qualified)
+// reference; braces still prefer it, so no rewrite.
+struct RefInitListCtor {
+ RefInitListCtor(Bar) {}
+ RefInitListCtor(const std::initializer_list<RefInitListCtor> &) {}
+};
+
+RefInitListCtor refInitListCtorNoRewrite(Bar b) {
+ return RefInitListCtor(b);
+}
+
Bar f8() {
return {};
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/205440
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
