https://github.com/gaul updated https://github.com/llvm/llvm-project/pull/237770
>From 9a50de4148c210200bc2fe779d7f8a3c4bf4ad80 Mon Sep 17 00:00:00 2001 From: Andrew Gaul <[email protected]> Date: Sun, 1 Nov 2026 12:35:24 -0800 Subject: [PATCH 1/2] [clang-tidy] Ignore 0 and NULL null pointers in modernize-use-emplace The check rewrites v.push_back(T(42, 0)); where the second parameter of the constructor of `T` is a pointer, to `v.emplace_back(42, 0)`. The forwarding reference of `emplace_back` deduces `0` as an `int`, which, unlike the literal, does not convert to a pointer, so the rewritten call does not compile. `NULL`, which is `__null` or an integer literal, has the same problem, as does a literal zero passed for a pointer to member. Ignore such calls, as the check already does for bit-field arguments, and remove the FIXME about them. `nullptr` is deduced as `std::nullptr_t`, which converts to any pointer, so calls passing it are still reported. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../clang-tidy/modernize/UseEmplaceCheck.cpp | 16 +++++++--- clang-tools-extra/docs/ReleaseNotes.md | 5 +++ .../checks/modernize/use-emplace.rst | 3 ++ .../checkers/modernize/use-emplace.cpp | 31 +++++++++++++++++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp index 8ec123fc476ff4..ea033e0794a995 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp @@ -181,6 +181,13 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) { ignoringImplicit(allOf(cxxConstructExpr(isListInitialization()), unless(cxxTemporaryObjectExpr())))); + // 0 and NULL convert to null pointers, but emplace_back would deduce them as + // integers, which don't. + auto NullPointerConstantAsArgument = hasAnyArgument( + expr(hasType(hasCanonicalType(anyOf(pointerType(), memberPointerType()))), + ignoringImplicit( + ignoringParenImpCasts(anyOf(integerLiteral(), gnuNullExpr()))))); + // We could have leak of resource. auto NewExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr())); // We would call another constructor. @@ -194,13 +201,14 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) { auto HasInitList = anyOf(has(ignoringImplicit(initListExpr())), has(cxxStdInitializerListExpr())); - // FIXME: Discard 0/NULL (as nullptr), static inline const data members, - // overloaded functions and template names. + // FIXME: Discard static inline const data members, overloaded functions and + // template names. const auto SoughtConstructExpr = cxxConstructExpr( unless(anyOf(IsCtorOfSmartPtr, HasInitList, BitFieldAsArgument, - InitializerListAsArgument, NewExprAsArgument, - ConstructingDerived, IsPrivateOrProtectedCtor))) + NullPointerConstantAsArgument, InitializerListAsArgument, + NewExprAsArgument, ConstructingDerived, + IsPrivateOrProtectedCtor))) .bind("ctor"); auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr)); diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 679ca9ecbb4a39..4cff278bdb91f8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -301,6 +301,11 @@ infrastructure are described first, followed by tool-specific sections. <clang-tidy/checks/modernize/use-designated-initializers>` when analyzing malformed code with nested classes and ambiguous initializer. +- Fixed false positive in {doc}`modernize-use-emplace + <clang-tidy/checks/modernize/use-emplace>` check when a constructor argument + is `0` or `NULL` used as a null pointer, which an `emplace`-like method would + forward as an integer. + - Improved {doc}`modernize-use-equals-default <clang-tidy/checks/modernize/use-equals-default>` check to also capture default constructor with explicit default-initialization of parents. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst index f1031f69a94679..bdcd6126298a83 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst @@ -111,6 +111,9 @@ Check also doesn't fire if any argument of the constructor call would be: - a bit-field (bit-fields can't bind to rvalue/universal reference) + - ``0`` or ``NULL`` converted to a null pointer (a universal reference + deduces them as integers, which don't convert to pointers) + - a ``new`` expression (to avoid leak) - if the argument would be converted via derived-to-base cast. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp index a2ab19efcf6981..ae920f991c39aa 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp @@ -196,6 +196,37 @@ void testNewExpr() { v.push_back(Derived(new int, Something{})); } +struct WithMember { + int Member; +}; + +struct MemberPointer { + MemberPointer(int WithMember::*) {} +}; + +void testNullPointerConstants() { + std::vector<Base> v; + v.push_back(Base(42, 0)); + v.push_back(Base(42, (0))); + v.push_back(Base(42, 0L)); + v.push_back(Base(42, __null)); + + v.push_back(Base(42, nullptr)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: v.emplace_back(42, nullptr); + + v.push_back(Base(0, nullptr, 0)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: v.emplace_back(0, nullptr, 0); + + std::vector<MemberPointer> w; + w.push_back(MemberPointer(0)); + + w.push_back(MemberPointer(nullptr)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: w.emplace_back(nullptr); +} + void testSpaces() { std::vector<Something> v; >From 17ebff87ca303f651519c42d9df3289d7d289a46 Mon Sep 17 00:00:00 2001 From: Andrew Gaul <[email protected]> Date: Sun, 1 Nov 2026 16:22:28 -0800 Subject: [PATCH 2/2] Add more null pointer test cases Cover a macro expanding to 0, explicit casts of 0, an emplace call with a temporary, and 0 passed as a function pointer, a member function pointer, a pointer through a type alias, and a pointer bound to a const reference. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../checkers/modernize/use-emplace.cpp | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp index ae920f991c39aa..d111d687dfb718 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp @@ -198,18 +198,41 @@ void testNewExpr() { struct WithMember { int Member; + void method(); }; struct MemberPointer { MemberPointer(int WithMember::*) {} }; +struct MemberFunctionPointer { + MemberFunctionPointer(void (WithMember::*)()) {} +}; + +struct FunctionPointer { + FunctionPointer(void (*)()) {} +}; + +using IntPointer = int *; + +struct PointerAlias { + PointerAlias(IntPointer) {} +}; + +struct PointerReference { + PointerReference(int *const &) {} +}; + +#define NULL_CONSTANT 0 + void testNullPointerConstants() { std::vector<Base> v; v.push_back(Base(42, 0)); v.push_back(Base(42, (0))); v.push_back(Base(42, 0L)); v.push_back(Base(42, __null)); + v.push_back(Base(42, NULL_CONSTANT)); + v.emplace_back(Base(42, 0)); v.push_back(Base(42, nullptr)); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back @@ -219,12 +242,44 @@ void testNullPointerConstants() { // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back // CHECK-FIXES: v.emplace_back(0, nullptr, 0); + v.push_back(Base(42, static_cast<int *>(0))); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: v.emplace_back(42, static_cast<int *>(0)); + + v.push_back(Base(42, (int *)0)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: v.emplace_back(42, (int *)0); + + v.emplace_back(Base(42, nullptr)); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v.emplace_back(42, nullptr); + std::vector<MemberPointer> w; w.push_back(MemberPointer(0)); w.push_back(MemberPointer(nullptr)); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back // CHECK-FIXES: w.emplace_back(nullptr); + + std::vector<MemberFunctionPointer> x; + x.push_back(MemberFunctionPointer(0)); + + x.push_back(MemberFunctionPointer(nullptr)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: x.emplace_back(nullptr); + + std::vector<FunctionPointer> y; + y.push_back(FunctionPointer(0)); + + y.push_back(FunctionPointer(nullptr)); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back + // CHECK-FIXES: y.emplace_back(nullptr); + + std::vector<PointerAlias> z; + z.push_back(PointerAlias(0)); + + std::vector<PointerReference> r; + r.push_back(PointerReference(0)); } void testSpaces() { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
