https://github.com/juanvazquez updated https://github.com/llvm/llvm-project/pull/186903
>From a95cdc8dc0c2c7c4aaa540838a08bae6b82efb93 Mon Sep 17 00:00:00 2001 From: Juan Vazquez <[email protected]> Date: Mon, 16 Mar 2026 22:44:32 +0100 Subject: [PATCH 1/5] [clang-tidy] use-after-move: Support null_after_move annotations Extend the bugprone-use-after-move check to recognize user defined smart-pointer-like types that make guarantees on the state of a moved-from object, leaving it in a valid and specified state that matches the standard smart pointer's moved-from state (nullptr), where it is safe to use but not dereference. Following the RFC discussion: * Use [[clang::annotate]] to mark the types. * Use an schema for the [[clang::annotate]] annotation and arguments to help avoid conflicts with other users of the attribute. * The annotation will identify the tool ("clang-tidy") and the arguments the plugin ("bugprone-use-after-move"), and the behavior of the type ("null_after_move"). E.g.: [[clang::annotate("clang-tidy", "bugprone-use-after-move", "null_after_move")]] RFC: https://discourse.llvm.org/t/rfc-add-a-class-attribute-clang-null-after-move-for-use-after-move-analysis/89760 --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 47 +++++++- .../checks/bugprone/use-after-move.rst | 7 +- .../checkers/bugprone/use-after-move.cpp | 101 ++++++++++++++++++ 3 files changed, 149 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 8e9f48ee0cd21..91f8bc06bf002 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -8,11 +8,14 @@ #include "UseAfterMoveCheck.h" +#include "clang/AST/Attr.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/Type.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" #include "clang/Analysis/CFG.h" +#include "clang/Basic/LLVM.h" #include "clang/Lex/Lexer.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" @@ -323,7 +326,32 @@ void UseAfterMoveFinder::getUsesAndReinits( }); } -static bool isStandardSmartPointer(const ValueDecl *VD) { +static std::optional<StringRef> getStringLiteral(const Expr *E) { + if (!E) + return std::nullopt; + if (const auto *SL = dyn_cast<StringLiteral>(E->IgnoreParenImpCasts())) + return SL->getString(); + return std::nullopt; +} + +// User defined types can use [[clang::annotate]] to mark smart-pointer-like +// types with a specified move from state that matches the standard smart +// pointer's moved-from state (nullptr). +static bool isNullAfterMoveAnnotate(const AnnotateAttr *Attr) { + if (Attr->getAnnotation() != "clang-tidy") + return false; + + if (Attr->args_size() != 2) + return false; + + std::optional<StringRef> Plugin = getStringLiteral(Attr->args_begin()[0]); + std::optional<StringRef> Annotation = getStringLiteral(Attr->args_begin()[1]); + + return Plugin && Annotation && *Plugin == "bugprone-use-after-move" && + *Annotation == "null_after_move"; +} + +static bool isSpecifiedAfterMove(const ValueDecl *VD) { const Type *TheType = VD->getType().getNonReferenceType().getTypePtrOrNull(); if (!TheType) return false; @@ -332,6 +360,16 @@ static bool isStandardSmartPointer(const ValueDecl *VD) { if (!RecordDecl) return false; + // Use the definition for the declaration, as it is the expected place to add + // the annotations. + const CXXRecordDecl *DefinitionDecl = RecordDecl->getDefinition(); + if (DefinitionDecl != nullptr) { + for (const auto *Attr : DefinitionDecl->specific_attrs<AnnotateAttr>()) + if (isNullAfterMoveAnnotate(Attr)) + return true; + } + + // Standard smart pointers have a well-specified moved-from state (nullptr). const IdentifierInfo *ID = RecordDecl->getIdentifier(); if (!ID) return false; @@ -358,9 +396,10 @@ void UseAfterMoveFinder::getDeclRefs( const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref"); const auto *Operator = Match.getNodeAs<CXXOperatorCallExpr>("operator"); if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) { - // Ignore uses of a standard smart pointer that don't dereference the - // pointer. - if (Operator || !isStandardSmartPointer(DeclRef->getDecl())) + // Ignore uses of a standard smart pointer or classes annotated as + // "null_after_move" (smart-pointer-like behavior) that don't + // dereference the pointer. + if (Operator || !isSpecifiedAfterMove(DeclRef->getDecl())) DeclRefs->insert(DeclRef); } } diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst index 3218b32ce2c58..a0e2cc767854d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst @@ -195,8 +195,6 @@ Use Any occurrence of the moved variable that is not a reinitialization (see below) is considered to be a use. -An exception to this are objects of type ``std::unique_ptr``, -``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``. An exception to this are objects of type ``std::unique_ptr``, ``std::shared_ptr``, ``std::weak_ptr``, ``std::optional``, and ``std::any``, which can be reinitialized via ``reset``. For smart pointers specifically, the @@ -204,6 +202,11 @@ moved-from objects have a well-defined state of being ``nullptr``s, and only ``operator*``, ``operator->`` and ``operator[]`` are considered bad accesses as they would be dereferencing a ``nullptr``. +User-defined types can be annotated as having the same semantics as standard +smart pointers with ``[[clang::annotate("clang-tidy", +"bugprone-use-after-move", "null_after_move")]]``. This expresses that a +moved-from object of this type is a null pointer. + If multiple uses occur after a move, only the first of these is flagged. Reinitialization diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp index 5d95c44fc318f..6a7b4a5614b88 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp @@ -1023,6 +1023,107 @@ void reinitAnnotation() { } } +//////////////////////////////////////////////////////////////////////////////// +// Tests for annotations on smart-pointer-like types + +namespace null_after_move { + +template <typename T> +class [[clang::annotate("clang-tidy", + "bugprone-use-after-move", + "null_after_move")]] SmartPtrAlike { +public: + SmartPtrAlike(); + ~SmartPtrAlike(); + SmartPtrAlike(const SmartPtrAlike&) = delete; + SmartPtrAlike &operator=(const SmartPtrAlike&) = delete; + SmartPtrAlike(SmartPtrAlike&& other); + SmartPtrAlike &operator=(SmartPtrAlike&& other); + T* get() const; + T& operator*() const; +}; + +// Don't flag uses of smart-pointer-like types correctly annotated, unless it's +// a dereference. +void smartPointerLikeTypeUseAfterMove() { + SmartPtrAlike<int> ptr; + ptr.get(); + SmartPtrAlike<int> other_ptr = std::move(ptr); + ptr.get(); + int inv_value = *ptr; + // CHECK-NOTES: [[@LINE-1]]:20: warning: 'ptr' used after it was moved + // CHECK-NOTES: [[@LINE-4]]:34: note: move occurred here +} + +class [[clang::annotate("other"), + clang::annotate("clang-tidy", + "bugprone-use-after-move", + "null_after_move")]] MultipleAnnotationsType { +public: + MultipleAnnotationsType(); + ~MultipleAnnotationsType(); + MultipleAnnotationsType(const MultipleAnnotationsType&) = delete; + MultipleAnnotationsType &operator=(const MultipleAnnotationsType&) = delete; + MultipleAnnotationsType(MultipleAnnotationsType&& other); + MultipleAnnotationsType &operator=(MultipleAnnotationsType&& other); + int* get() const; + int& operator*() const; +}; + +// Handle smart-pointer-like types correctly annotated, even in the case of +// multiple annotations. +void typeWithMultipleAnnotations() { + MultipleAnnotationsType ptr; + ptr.get(); + MultipleAnnotationsType other_ptr = std::move(ptr); + ptr.get(); + int inv_value = *ptr; + // CHECK-NOTES: [[@LINE-1]]:20: warning: 'ptr' used after it was moved + // CHECK-NOTES: [[@LINE-4]]:39: note: move occurred here +} + +class [[clang::annotate("null_after_move")]] BadAnnotation { +public: + BadAnnotation(); + ~BadAnnotation(); + BadAnnotation(const BadAnnotation&) = delete; + BadAnnotation &operator=(const BadAnnotation&) = delete; + BadAnnotation(BadAnnotation&& other); + BadAnnotation &operator=(BadAnnotation&& other); + int* get() const; +}; + +// Flag uses of smart-pointer-like types with incorrect annotate. +void badUseAfterMoveAnnotationIgnored() { + BadAnnotation ptr; + BadAnnotation other_ptr = std::move(ptr); + ptr.get(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'ptr' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:29: note: move occurred here +} + +class [[clang::annotate("clang-tidy", "null_after_move")]] BadAnnotationArgs { +public: + BadAnnotationArgs(); + ~BadAnnotationArgs(); + BadAnnotationArgs(const BadAnnotationArgs&) = delete; + BadAnnotationArgs &operator=(const BadAnnotationArgs&) = delete; + BadAnnotationArgs(BadAnnotationArgs&& other); + BadAnnotationArgs &operator=(BadAnnotationArgs&& other); + int* get() const; +}; + +// Flag uses of smart-pointer-like types with wrong annotate arguments. +void UseAfterMoveAnnotationWithBadArgsIgnored() { + BadAnnotationArgs ptr; + BadAnnotationArgs other_ptr = std::move(ptr); + ptr.get(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'ptr' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:33: note: move occurred here +} + +} // namespace null_after_move + //////////////////////////////////////////////////////////////////////////////// // Tests related to order of evaluation within expressions >From 555d604a2df2d2c42c67206c7b7270ef6e81f4b9 Mon Sep 17 00:00:00 2001 From: Juan Vazquez <[email protected]> Date: Tue, 17 Mar 2026 09:37:22 +0100 Subject: [PATCH 2/5] Add release note for bugprone-use-after-move --- clang-tools-extra/docs/ReleaseNotes.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c9a170a9e8660..92c876ba1d12a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -232,9 +232,14 @@ Changes in existing checks function is unsafe, useless, deprecated in C++17 and removed in C++20). - Improved :doc:`bugprone-use-after-move - <clang-tidy/checks/bugprone/use-after-move>` check by including the name of - the invalidating function in the warning message when a custom invalidation - function is used (via the `InvalidationFunctions` option). + <clang-tidy/checks/bugprone/use-after-move>` check: + + - Include the name of the invalidating function in the warning message when a + custom invalidation function is used (via the `InvalidationFunctions` + option). + + - Add support to annotate user-defined types as having the same moved-from + semantics as standard smart pointers. - Improved :doc:`cppcoreguidelines-init-variables <clang-tidy/checks/cppcoreguidelines/init-variables>` check by ensuring that @@ -357,7 +362,7 @@ Changes in existing checks - Diagnose and remove redundant ``else`` branches after calls to ``[[noreturn]]`` functions. - + - Improved :doc:`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>` check: the warning message now uses separate note diagnostics for each uninitialized enumerator, making >From 54ba797822454ec3ea2c3c8ed5cf8bbdd7ddfd7b Mon Sep 17 00:00:00 2001 From: Juan Vazquez <[email protected]> Date: Tue, 17 Mar 2026 21:31:16 +0100 Subject: [PATCH 3/5] Fix release note grammar --- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 92c876ba1d12a..2c670c7db9d07 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -238,8 +238,8 @@ Changes in existing checks custom invalidation function is used (via the `InvalidationFunctions` option). - - Add support to annotate user-defined types as having the same moved-from - semantics as standard smart pointers. + - Add support for annotation of user-defined types as having the same + moved-from semantics as standard smart pointers. - Improved :doc:`cppcoreguidelines-init-variables <clang-tidy/checks/cppcoreguidelines/init-variables>` check by ensuring that >From 4290c467b6a28291b44a6722c272ee147e73845b Mon Sep 17 00:00:00 2001 From: Juan Vazquez <[email protected]> Date: Sat, 21 Mar 2026 17:49:31 +0100 Subject: [PATCH 4/5] Address review feedback --- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 91f8bc06bf002..6b86483f821a8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -11,11 +11,9 @@ #include "clang/AST/Attr.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" -#include "clang/AST/Type.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" #include "clang/Analysis/CFG.h" -#include "clang/Basic/LLVM.h" #include "clang/Lex/Lexer.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" @@ -363,7 +361,7 @@ static bool isSpecifiedAfterMove(const ValueDecl *VD) { // Use the definition for the declaration, as it is the expected place to add // the annotations. const CXXRecordDecl *DefinitionDecl = RecordDecl->getDefinition(); - if (DefinitionDecl != nullptr) { + if (DefinitionDecl) { for (const auto *Attr : DefinitionDecl->specific_attrs<AnnotateAttr>()) if (isNullAfterMoveAnnotate(Attr)) return true; >From 8cfe499c7bca414e11d74b1cd03c6a71a96fc52b Mon Sep 17 00:00:00 2001 From: Juan Vazquez <[email protected]> Date: Sun, 22 Mar 2026 11:51:17 +0100 Subject: [PATCH 5/5] Address review feedback --- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 6b86483f821a8..06b5940c648a3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -325,8 +325,7 @@ void UseAfterMoveFinder::getUsesAndReinits( } static std::optional<StringRef> getStringLiteral(const Expr *E) { - if (!E) - return std::nullopt; + assert(E); if (const auto *SL = dyn_cast<StringLiteral>(E->IgnoreParenImpCasts())) return SL->getString(); return std::nullopt; @@ -360,8 +359,7 @@ static bool isSpecifiedAfterMove(const ValueDecl *VD) { // Use the definition for the declaration, as it is the expected place to add // the annotations. - const CXXRecordDecl *DefinitionDecl = RecordDecl->getDefinition(); - if (DefinitionDecl) { + if (const CXXRecordDecl *DefinitionDecl = RecordDecl->getDefinition()) { for (const auto *Attr : DefinitionDecl->specific_attrs<AnnotateAttr>()) if (isNullAfterMoveAnnotate(Attr)) return true; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
