Skip to content
Merged
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "CommandProcessorCheck.h"
#include "ComparePointerToMemberVirtualFunctionCheck.h"
#include "CopyConstructorInitCheck.h"
#include "CopyConstructorMutatesArgumentCheck.h"
#include "CrtpConstructorAccessibilityCheck.h"
#include "DanglingHandleCheck.h"
#include "DerivedMethodShadowingBaseMethodCheck.h"
Expand Down Expand Up @@ -198,6 +199,8 @@ class BugproneModule : public ClangTidyModule {
"bugprone-multiple-new-in-one-expression");
CheckFactories.registerCheck<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
CheckFactories.registerCheck<CopyConstructorMutatesArgumentCheck>(
"bugprone-copy-constructor-mutates-argument");
CheckFactories.registerCheck<NondeterministicPointerIterationOrderCheck>(
"bugprone-nondeterministic-pointer-iteration-order");
CheckFactories.registerCheck<OptionalValueConversionCheck>(
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ add_clang_library(clangTidyBugproneModule STATIC
CommandProcessorCheck.cpp
ComparePointerToMemberVirtualFunctionCheck.cpp
CopyConstructorInitCheck.cpp
CopyConstructorMutatesArgumentCheck.cpp
CrtpConstructorAccessibilityCheck.cpp
DanglingHandleCheck.cpp
DerivedMethodShadowingBaseMethodCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@
//
//===----------------------------------------------------------------------===//

#include "MutatingCopyCheck.h"
#include "CopyConstructorMutatesArgumentCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"

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";
static constexpr llvm::StringLiteral MutatingCallName = "MutatingCall";

void MutatingCopyCheck::registerMatchers(MatchFinder *Finder) {
void CopyConstructorMutatesArgumentCheck::registerMatchers(
MatchFinder *Finder) {
const auto MemberExprOrSourceObject = anyOf(
memberExpr(),
declRefExpr(to(decl(equalsBoundNode(std::string(SourceDeclName))))));
Expand Down Expand Up @@ -60,7 +61,8 @@ void MutatingCopyCheck::registerMatchers(MatchFinder *Finder) {
this);
}

void MutatingCopyCheck::check(const MatchFinder::MatchResult &Result) {
void CopyConstructorMutatesArgumentCheck::check(
const MatchFinder::MatchResult &Result) {
if (const auto *MemberCall =
Result.Nodes.getNodeAs<CXXMemberCallExpr>(MutatingCallName))
diag(MemberCall->getBeginLoc(), "call mutates copied object");
Expand All @@ -69,4 +71,4 @@ void MutatingCopyCheck::check(const MatchFinder::MatchResult &Result) {
diag(Assignment->getBeginLoc(), "mutating copied object");
}

} // namespace clang::tidy::cert
} // namespace clang::tidy::bugprone
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@
//
//===----------------------------------------------------------------------===//

#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_COPYCONSTRUCTORMUTATESARGUMENTCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_COPYCONSTRUCTORMUTATESARGUMENTCHECK_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
class MutatingCopyCheck : public ClangTidyCheck {
/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/copy-constructor-mutates-argument.html
class CopyConstructorMutatesArgumentCheck : public ClangTidyCheck {
public:
MutatingCopyCheck(StringRef Name, ClangTidyContext *Context)
CopyConstructorMutatesArgumentCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
Expand All @@ -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_COPYCONSTRUCTORMUTATESARGUMENTCHECK_H
5 changes: 3 additions & 2 deletions clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "../bugprone/BadSignalToKillThreadCheck.h"
#include "../bugprone/CommandProcessorCheck.h"
#include "../bugprone/CopyConstructorMutatesArgumentCheck.h"
#include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
#include "../bugprone/ReservedIdentifierCheck.h"
#include "../bugprone/SignalHandlerCheck.h"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -280,7 +280,8 @@ class CERTModule : public ClangTidyModule {
"cert-oop54-cpp");
CheckFactories.registerCheck<NonTrivialTypesLibcMemoryCallsCheck>(
"cert-oop57-cpp");
CheckFactories.registerCheck<MutatingCopyCheck>("cert-oop58-cpp");
CheckFactories.registerCheck<bugprone::CopyConstructorMutatesArgumentCheck>(
"cert-oop58-cpp");

// C checkers
// ARR
Expand Down
1 change: 0 additions & 1 deletion clang-tools-extra/clang-tidy/cert/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ add_clang_library(clangTidyCERTModule STATIC
DontModifyStdNamespaceCheck.cpp
FloatLoopCounter.cpp
LimitedRandomnessCheck.cpp
MutatingCopyCheck.cpp
NonTrivialTypesLibcMemoryCallsCheck.cpp
ProperlySeededRandomGeneratorCheck.cpp
ThrownExceptionTypeCheck.cpp
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,11 @@ New check aliases
<clang-tidy/checks/bugprone/throwing-static-initialization>`
keeping initial check as an alias to the new one.

- Renamed :doc:`cert-oop58-cpp <clang-tidy/checks/cert/oop58-cpp>` to
:doc:`bugprone-copy-constructor-mutates-argument
<clang-tidy/checks/bugprone/copy-constructor-mutates-argument>`
keeping initial check as an alias to the new one.

Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.. title:: clang-tidy - bugprone-copy-constructor-mutates-argument

bugprone-copy-constructor-mutates-argument
==========================================

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>`_.
13 changes: 6 additions & 7 deletions clang-tools-extra/docs/clang-tidy/checks/cert/oop58-cpp.rst
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
.. title:: clang-tidy - cert-mutating-copy
.. title:: clang-tidy - cert-oop58-cpp
.. meta::
:http-equiv=refresh: 5;URL=../bugprone/copy-constructor-mutates-argument.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-copy-constructor-mutates-argument <../bugprone/copy-constructor-mutates-argument>`
for more information.
3 changes: 2 additions & 1 deletion clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ Clang-Tidy Checks
:doc:`bugprone-command-processor <bugprone/command-processor>`,
:doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
:doc:`bugprone-copy-constructor-init <bugprone/copy-constructor-init>`, "Yes"
:doc:`bugprone-copy-constructor-mutates-argument <bugprone/copy-constructor-mutates-argument>`,
:doc:`bugprone-crtp-constructor-accessibility <bugprone/crtp-constructor-accessibility>`, "Yes"
:doc:`bugprone-dangling-handle <bugprone/dangling-handle>`,
:doc:`bugprone-derived-method-shadowing-base-method <bugprone/derived-method-shadowing-base-method>`,
Expand Down Expand Up @@ -180,7 +181,6 @@ Clang-Tidy Checks
:doc:`cert-mem57-cpp <cert/mem57-cpp>`,
:doc:`cert-msc50-cpp <cert/msc50-cpp>`,
:doc:`cert-msc51-cpp <cert/msc51-cpp>`,
:doc:`cert-oop57-cpp <cert/oop57-cpp>`,
:doc:`cert-oop58-cpp <cert/oop58-cpp>`,
:doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`,
:doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`,
Expand Down Expand Up @@ -458,6 +458,7 @@ Check aliases
:doc:`cert-msc54-cpp <cert/msc54-cpp>`, :doc:`bugprone-signal-handler <bugprone/signal-handler>`,
:doc:`cert-oop11-cpp <cert/oop11-cpp>`, :doc:`performance-move-constructor-init <performance/move-constructor-init>`,
:doc:`cert-oop54-cpp <cert/oop54-cpp>`, :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`,
:doc:`cert-oop58-cpp <cert/oop58-cpp>`, :doc:`bugprone-copy-constructor-mutates-argument <bugprone/copy-constructor-mutates-argument>`,
:doc:`cert-pos44-c <cert/pos44-c>`, :doc:`bugprone-bad-signal-to-kill-thread <bugprone/bad-signal-to-kill-thread>`,
:doc:`cert-pos47-c <cert/pos47-c>`, :doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`,
:doc:`cert-sig30-c <cert/sig30-c>`, :doc:`bugprone-signal-handler <bugprone/signal-handler>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %check_clang_tidy %s cert-oop58-cpp %t
// RUN: %check_clang_tidy %s bugprone-copy-constructor-mutates-argument %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
Expand Down