[clang-tidy] Rename and move 'cert-oop58-cpp' to 'bugprone-copy-constructor-mutates-argument' #164566
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Member
|
@llvm/pr-subscribers-clang-tidy Author: Dasha Buka (dvbuka) Changescloses #157293 ✅ Changes:
🔧 Local testing:
Full diff: https://github.com/llvm/llvm-project/pull/164566.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index e6115f67656bc..79851be690759 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -52,6 +52,7 @@
#include "MultiLevelImplicitPointerConversionCheck.h"
#include "MultipleNewInOneExpressionCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "MutatingCopyCheck.h"
#include "NarrowingConversionsCheck.h"
#include "NoEscapeCheck.h"
#include "NonZeroEnumToBoolConversionCheck.h"
@@ -198,6 +199,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-multiple-new-in-one-expression");
CheckFactories.registerCheck<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
+ CheckFactories.registerCheck<MutatingCopyCheck>("bugprone-mutating-copy");
CheckFactories.registerCheck<NondeterministicPointerIterationOrderCheck>(
"bugprone-nondeterministic-pointer-iteration-order");
CheckFactories.registerCheck<OptionalValueConversionCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index c8943e5b22ef8..0dfcd3270cac2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -53,6 +53,7 @@ add_clang_library(clangTidyBugproneModule STATIC
MultiLevelImplicitPointerConversionCheck.cpp
MultipleNewInOneExpressionCheck.cpp
MultipleStatementMacroCheck.cpp
+ MutatingCopyCheck.cpp
NarrowingConversionsCheck.cpp
NoEscapeCheck.cpp
NonZeroEnumToBoolConversionCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.cpp
similarity index 97%
rename from clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
rename to clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.cpp
index fb9d72ce6bd31..9485270864618 100644
--- a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.cpp
@@ -12,7 +12,7 @@
using namespace clang::ast_matchers;
-namespace clang::tidy::cert {
+namespace clang::tidy::bugprone {
static constexpr llvm::StringLiteral SourceDeclName = "ChangedPVD";
static constexpr llvm::StringLiteral MutatingOperatorName = "MutatingOp";
@@ -69,4 +69,4 @@ void MutatingCopyCheck::check(const MatchFinder::MatchResult &Result) {
diag(Assignment->getBeginLoc(), "mutating copied object");
}
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.h
similarity index 73%
rename from clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
rename to clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.h
index c211fa004120c..9d2c8f12f65fa 100644
--- a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.h
@@ -6,18 +6,18 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MUTATINGCOPYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MUTATINGCOPYCHECK_H
#include "../ClangTidyCheck.h"
-namespace clang::tidy::cert {
+namespace clang::tidy::bugprone {
/// Finds assignments to the copied object and its direct or indirect members
/// in copy constructors and copy assignment operators.
///
/// For the user-facing documentation see:
-/// https://clang.llvm.org/extra/clang-tidy/checks/cert/oop58-cpp.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/mutating-copy.html
class MutatingCopyCheck : public ClangTidyCheck {
public:
MutatingCopyCheck(StringRef Name, ClangTidyContext *Context)
@@ -29,6 +29,6 @@ class MutatingCopyCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::bugprone
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MUTATINGCOPYCHECK_H
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index c1ca2cec7a1eb..01f33371f52cc 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "../bugprone/BadSignalToKillThreadCheck.h"
#include "../bugprone/CommandProcessorCheck.h"
+#include "../bugprone/MutatingCopyCheck.h"
#include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
#include "../bugprone/ReservedIdentifierCheck.h"
#include "../bugprone/SignalHandlerCheck.h"
@@ -38,7 +39,6 @@
#include "DontModifyStdNamespaceCheck.h"
#include "FloatLoopCounter.h"
#include "LimitedRandomnessCheck.h"
-#include "MutatingCopyCheck.h"
#include "NonTrivialTypesLibcMemoryCallsCheck.h"
#include "ProperlySeededRandomGeneratorCheck.h"
#include "ThrownExceptionTypeCheck.h"
@@ -280,7 +280,7 @@ class CERTModule : public ClangTidyModule {
"cert-oop54-cpp");
CheckFactories.registerCheck<NonTrivialTypesLibcMemoryCallsCheck>(
"cert-oop57-cpp");
- CheckFactories.registerCheck<MutatingCopyCheck>("cert-oop58-cpp");
+ CheckFactories.registerCheck<bugprone::MutatingCopyCheck>("cert-oop58-cpp");
// C checkers
// ARR
diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
index 453d1d30921e9..0feaf204f8ef4 100644
--- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -9,7 +9,6 @@ add_clang_library(clangTidyCERTModule STATIC
DontModifyStdNamespaceCheck.cpp
FloatLoopCounter.cpp
LimitedRandomnessCheck.cpp
- MutatingCopyCheck.cpp
NonTrivialTypesLibcMemoryCallsCheck.cpp
ProperlySeededRandomGeneratorCheck.cpp
ThrownExceptionTypeCheck.cpp
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/mutating-copy.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/mutating-copy.rst
new file mode 100644
index 0000000000000..137659ea43791
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/mutating-copy.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - bugprone-mutating-copy
+
+bugprone-mutating-copy
+======================
+
+Finds assignments to the copied object and its direct or indirect members
+in copy constructors and copy assignment operators.
+
+This check corresponds to the CERT C Coding Standard rule
+`OOP58-CPP. Copy operations must not mutate the source object
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object>`_.
\ No newline at end of file
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
index 399fb1b7e9279..6f16d9c548279 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
@@ -1,11 +1,10 @@
.. title:: clang-tidy - cert-mutating-copy
+.. meta::
+ :http-equiv=refresh: 5;URL=../bugprone/mutating-copy.html
cert-oop58-cpp
==============
-Finds assignments to the copied object and its direct or indirect members
-in copy constructors and copy assignment operators.
-
-This check corresponds to the CERT C Coding Standard rule
-`OOP58-CPP. Copy operations must not mutate the source object
-<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object>`_.
+The `cert-oop58-cpp` check is an alias, please see
+:doc:`bugprone-mutating-copy <../bugprone/mutating-copy>`
+for more information.
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/mutating-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/mutating-copy.cpp
new file mode 100644
index 0000000000000..8b93aa3ee54fc
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/mutating-copy.cpp
@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s bugprone-mutating-copy %t
+
+// Example test cases from CERT rule
+// https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object
+namespace test_mutating_noncompliant_example {
+class A {
+ mutable int m;
+
+public:
+ A() : m(0) {}
+ explicit A(int m) : m(m) {}
+
+ A(const A &other) : m(other.m) {
+ other.m = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ }
+
+ A &operator=(const A &other) {
+ if (&other != this) {
+ m = other.m;
+ other.m = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: mutating copied object
+ }
+ return *this;
+ }
+
+ int get_m() const { return m; }
+};
+} // namespace test_mutating_noncompliant_example
+
+namespace test_mutating_compliant_example {
+class B {
+ int m;
+
+public:
+ B() : m(0) {}
+ explicit B(int m) : m(m) {}
+
+ B(const B &other) : m(other.m) {}
+ B(B &&other) : m(other.m) {
+ other.m = 0; //no-warning: mutation allowed in move constructor
+ }
+
+ B &operator=(const B &other) {
+ if (&other != this) {
+ m = other.m;
+ }
+ return *this;
+ }
+
+ B &operator=(B &&other) {
+ m = other.m;
+ other.m = 0; //no-warning: mutation allowed in move assignment operator
+ return *this;
+ }
+
+ int get_m() const { return m; }
+};
+} // namespace test_mutating_compliant_example
+
+namespace test_mutating_pointer {
+class C {
+ C *ptr;
+ int value;
+
+ C();
+ C(C &other) {
+ other = {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ other.ptr = nullptr;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ other.value = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+
+ // no-warning: mutating a pointee is allowed
+ other.ptr->value = 0;
+ *other.ptr = {};
+ }
+};
+} // namespace test_mutating_pointer
+
+namespace test_mutating_indirect_member {
+struct S {
+ int x;
+};
+
+class D {
+ S s;
+ D(D &other) {
+ other.s = {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ other.s.x = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ }
+};
+} // namespace test_mutating_indirect_member
+
+namespace test_mutating_other_object {
+class E {
+ E();
+ E(E &other) {
+ E tmp;
+ // no-warning: mutating an object that is not the source is allowed
+ tmp = {};
+ }
+};
+} // namespace test_mutating_other_object
+
+namespace test_mutating_member_function {
+class F {
+ int a;
+
+public:
+ void bad_func() { a = 12; }
+ void fine_func() const;
+ void fine_func_2(int x) { x = 5; }
+ void questionable_func();
+
+ F(F &other) : a(other.a) {
+ this->bad_func(); // no-warning: mutating this is allowed
+
+ other.bad_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+
+ other.fine_func();
+ other.fine_func_2(42);
+ other.questionable_func();
+ }
+};
+} // namespace test_mutating_member_function
+
+namespace test_mutating_function_on_nested_object {
+struct S {
+ int x;
+ void mutate(int y) {
+ x = y;
+ }
+};
+
+class G {
+ S s;
+ G(G &other) {
+ s.mutate(0); // no-warning: mutating this is allowed
+
+ other.s.mutate(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+ }
+};
+} // namespace test_mutating_function_on_nested_object
|
Member
|
@llvm/pr-subscribers-clang-tools-extra Author: Dasha Buka (dvbuka) Changescloses #157293 ✅ Changes:
🔧 Local testing:
Full diff: https://github.com/llvm/llvm-project/pull/164566.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index e6115f67656bc..79851be690759 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -52,6 +52,7 @@
#include "MultiLevelImplicitPointerConversionCheck.h"
#include "MultipleNewInOneExpressionCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "MutatingCopyCheck.h"
#include "NarrowingConversionsCheck.h"
#include "NoEscapeCheck.h"
#include "NonZeroEnumToBoolConversionCheck.h"
@@ -198,6 +199,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-multiple-new-in-one-expression");
CheckFactories.registerCheck<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
+ CheckFactories.registerCheck<MutatingCopyCheck>("bugprone-mutating-copy");
CheckFactories.registerCheck<NondeterministicPointerIterationOrderCheck>(
"bugprone-nondeterministic-pointer-iteration-order");
CheckFactories.registerCheck<OptionalValueConversionCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index c8943e5b22ef8..0dfcd3270cac2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -53,6 +53,7 @@ add_clang_library(clangTidyBugproneModule STATIC
MultiLevelImplicitPointerConversionCheck.cpp
MultipleNewInOneExpressionCheck.cpp
MultipleStatementMacroCheck.cpp
+ MutatingCopyCheck.cpp
NarrowingConversionsCheck.cpp
NoEscapeCheck.cpp
NonZeroEnumToBoolConversionCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.cpp
similarity index 97%
rename from clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
rename to clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.cpp
index fb9d72ce6bd31..9485270864618 100644
--- a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.cpp
@@ -12,7 +12,7 @@
using namespace clang::ast_matchers;
-namespace clang::tidy::cert {
+namespace clang::tidy::bugprone {
static constexpr llvm::StringLiteral SourceDeclName = "ChangedPVD";
static constexpr llvm::StringLiteral MutatingOperatorName = "MutatingOp";
@@ -69,4 +69,4 @@ void MutatingCopyCheck::check(const MatchFinder::MatchResult &Result) {
diag(Assignment->getBeginLoc(), "mutating copied object");
}
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.h
similarity index 73%
rename from clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
rename to clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.h
index c211fa004120c..9d2c8f12f65fa 100644
--- a/clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/MutatingCopyCheck.h
@@ -6,18 +6,18 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MUTATINGCOPYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MUTATINGCOPYCHECK_H
#include "../ClangTidyCheck.h"
-namespace clang::tidy::cert {
+namespace clang::tidy::bugprone {
/// Finds assignments to the copied object and its direct or indirect members
/// in copy constructors and copy assignment operators.
///
/// For the user-facing documentation see:
-/// https://clang.llvm.org/extra/clang-tidy/checks/cert/oop58-cpp.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/mutating-copy.html
class MutatingCopyCheck : public ClangTidyCheck {
public:
MutatingCopyCheck(StringRef Name, ClangTidyContext *Context)
@@ -29,6 +29,6 @@ class MutatingCopyCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::bugprone
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MUTATINGCOPYCHECK_H
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index c1ca2cec7a1eb..01f33371f52cc 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "../bugprone/BadSignalToKillThreadCheck.h"
#include "../bugprone/CommandProcessorCheck.h"
+#include "../bugprone/MutatingCopyCheck.h"
#include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
#include "../bugprone/ReservedIdentifierCheck.h"
#include "../bugprone/SignalHandlerCheck.h"
@@ -38,7 +39,6 @@
#include "DontModifyStdNamespaceCheck.h"
#include "FloatLoopCounter.h"
#include "LimitedRandomnessCheck.h"
-#include "MutatingCopyCheck.h"
#include "NonTrivialTypesLibcMemoryCallsCheck.h"
#include "ProperlySeededRandomGeneratorCheck.h"
#include "ThrownExceptionTypeCheck.h"
@@ -280,7 +280,7 @@ class CERTModule : public ClangTidyModule {
"cert-oop54-cpp");
CheckFactories.registerCheck<NonTrivialTypesLibcMemoryCallsCheck>(
"cert-oop57-cpp");
- CheckFactories.registerCheck<MutatingCopyCheck>("cert-oop58-cpp");
+ CheckFactories.registerCheck<bugprone::MutatingCopyCheck>("cert-oop58-cpp");
// C checkers
// ARR
diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
index 453d1d30921e9..0feaf204f8ef4 100644
--- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -9,7 +9,6 @@ add_clang_library(clangTidyCERTModule STATIC
DontModifyStdNamespaceCheck.cpp
FloatLoopCounter.cpp
LimitedRandomnessCheck.cpp
- MutatingCopyCheck.cpp
NonTrivialTypesLibcMemoryCallsCheck.cpp
ProperlySeededRandomGeneratorCheck.cpp
ThrownExceptionTypeCheck.cpp
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/mutating-copy.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/mutating-copy.rst
new file mode 100644
index 0000000000000..137659ea43791
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/mutating-copy.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - bugprone-mutating-copy
+
+bugprone-mutating-copy
+======================
+
+Finds assignments to the copied object and its direct or indirect members
+in copy constructors and copy assignment operators.
+
+This check corresponds to the CERT C Coding Standard rule
+`OOP58-CPP. Copy operations must not mutate the source object
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object>`_.
\ No newline at end of file
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
index 399fb1b7e9279..6f16d9c548279 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
@@ -1,11 +1,10 @@
.. title:: clang-tidy - cert-mutating-copy
+.. meta::
+ :http-equiv=refresh: 5;URL=../bugprone/mutating-copy.html
cert-oop58-cpp
==============
-Finds assignments to the copied object and its direct or indirect members
-in copy constructors and copy assignment operators.
-
-This check corresponds to the CERT C Coding Standard rule
-`OOP58-CPP. Copy operations must not mutate the source object
-<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object>`_.
+The `cert-oop58-cpp` check is an alias, please see
+:doc:`bugprone-mutating-copy <../bugprone/mutating-copy>`
+for more information.
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/mutating-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/mutating-copy.cpp
new file mode 100644
index 0000000000000..8b93aa3ee54fc
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/mutating-copy.cpp
@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s bugprone-mutating-copy %t
+
+// Example test cases from CERT rule
+// https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object
+namespace test_mutating_noncompliant_example {
+class A {
+ mutable int m;
+
+public:
+ A() : m(0) {}
+ explicit A(int m) : m(m) {}
+
+ A(const A &other) : m(other.m) {
+ other.m = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ }
+
+ A &operator=(const A &other) {
+ if (&other != this) {
+ m = other.m;
+ other.m = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: mutating copied object
+ }
+ return *this;
+ }
+
+ int get_m() const { return m; }
+};
+} // namespace test_mutating_noncompliant_example
+
+namespace test_mutating_compliant_example {
+class B {
+ int m;
+
+public:
+ B() : m(0) {}
+ explicit B(int m) : m(m) {}
+
+ B(const B &other) : m(other.m) {}
+ B(B &&other) : m(other.m) {
+ other.m = 0; //no-warning: mutation allowed in move constructor
+ }
+
+ B &operator=(const B &other) {
+ if (&other != this) {
+ m = other.m;
+ }
+ return *this;
+ }
+
+ B &operator=(B &&other) {
+ m = other.m;
+ other.m = 0; //no-warning: mutation allowed in move assignment operator
+ return *this;
+ }
+
+ int get_m() const { return m; }
+};
+} // namespace test_mutating_compliant_example
+
+namespace test_mutating_pointer {
+class C {
+ C *ptr;
+ int value;
+
+ C();
+ C(C &other) {
+ other = {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ other.ptr = nullptr;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ other.value = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+
+ // no-warning: mutating a pointee is allowed
+ other.ptr->value = 0;
+ *other.ptr = {};
+ }
+};
+} // namespace test_mutating_pointer
+
+namespace test_mutating_indirect_member {
+struct S {
+ int x;
+};
+
+class D {
+ S s;
+ D(D &other) {
+ other.s = {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ other.s.x = 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+ }
+};
+} // namespace test_mutating_indirect_member
+
+namespace test_mutating_other_object {
+class E {
+ E();
+ E(E &other) {
+ E tmp;
+ // no-warning: mutating an object that is not the source is allowed
+ tmp = {};
+ }
+};
+} // namespace test_mutating_other_object
+
+namespace test_mutating_member_function {
+class F {
+ int a;
+
+public:
+ void bad_func() { a = 12; }
+ void fine_func() const;
+ void fine_func_2(int x) { x = 5; }
+ void questionable_func();
+
+ F(F &other) : a(other.a) {
+ this->bad_func(); // no-warning: mutating this is allowed
+
+ other.bad_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+
+ other.fine_func();
+ other.fine_func_2(42);
+ other.questionable_func();
+ }
+};
+} // namespace test_mutating_member_function
+
+namespace test_mutating_function_on_nested_object {
+struct S {
+ int x;
+ void mutate(int y) {
+ x = y;
+ }
+};
+
+class G {
+ S s;
+ G(G &other) {
+ s.mutate(0); // no-warning: mutating this is allowed
+
+ other.s.mutate(0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+ }
+};
+} // namespace test_mutating_function_on_nested_object
|
dvbuka
changed the title
Contributor
|
Please don't place AI generated PR descriptions. It's just white noise. |
Contributor
|
Also your diff is different then other PR. I think test files weren't moved bug copied instead |
Contributor
|
|
EugeneZelenko
requested review from
HerrCai0907,
carlosgalvezp,
localspook and
vbvictor
|
✅ With the latest revision this PR passed the C/C++ code linter. |
dvbuka
changed the title
vbvictor
approved these changes
Nov 2, 2025
Contributor
vbvictor
left a comment
vbvictor
left a comment
There was a problem hiding this comment.
LGTM if conflicts are resolved
vbvictor
reviewed
Nov 2, 2025
localspook
approved these changes
Nov 7, 2025
|
@dvbuka Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/193/builds/11960 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/38/builds/6235 Here is the relevant piece of the build log for the reference |
vinay-deshmukh
pushed a commit
to vinay-deshmukh/llvm-project
that referenced
this pull request
Nov 8, 2025
…ructor-mutates-argument' (llvm#164566) closes llvm#157293 --------- Co-authored-by: Baranov Victor <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #157293