8000 [clang-tidy] Add check bugprone-misleading-setter-of-reference (#132242) · llvm/llvm-project@d84b97e · GitHub
[go: up one dir, main page]
Skip to content

Commit d84b97e

Browse files
balazskebalazske
authored
[clang-tidy] Add check bugprone-misleading-setter-of-reference (#132242)
1 parent 8d0ebd8 commit d84b97e

File tree

8 files changed

+276
-0
lines changed

8 files changed

+276
-0
lines changed

‎clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "LambdaFunctionNameCheck.h"
4242
#include "MacroParenthesesCheck.h"
4343
#include "MacroRepeatedSideEffectsCheck.h"
44+
#include "MisleadingSetterOfReferenceCheck.h"
4445
#include "MisplacedOperatorInStrlenInAllocCheck.h"
4546
#include "MisplacedPointerArithmeticInAllocCheck.h"
4647
#include "MisplacedWideningCastCheck.h"
@@ -170,6 +171,8 @@ class BugproneModule : public ClangTidyModule {
170171
"bugprone-macro-parentheses");
171172
CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
172173
"bugprone-macro-repeated-side-effects");
174+
CheckFactories.registerCheck<MisleadingSetterOfReferenceCheck>(
175+
"bugprone-misleading-setter-of-reference");
173176
CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>(
174177
"bugprone-misplaced-operator-in-strlen-in-alloc");
175178
CheckFactories.registerCheck<MisplacedPointerArithmeticInAllocCheck>(

‎clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ add_clang_library(clangTidyBugproneModule STATIC
4242
LambdaFunctionNameCheck.cpp
4343
MacroParenthesesCheck.cpp
4444
MacroRepeatedSideEffectsCheck.cpp
45+
MisleadingSetterOfReferenceCheck.cpp
4546
MisplacedOperatorInStrlenInAllocCheck.cpp
4647
MisplacedPointerArithmeticInAllocCheck.cpp
4748
MisplacedWideningCastCheck.cpp

‎clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
//===--- MisleadingSetterOfReferenceCheck.cpp - clang-tidy-----------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "MisleadingSetterOfReferenceCheck.h"
10+
#include "clang/AST/ASTContext.h"
11+
#include "clang/ASTMatchers/ASTMatchFinder.h"
12+
13+
using namespace clang::ast_matchers;
14+
15+
namespace clang::tidy::bugprone {
16+
17+
void MisleadingSetterOfReferenceCheck::registerMatchers(MatchFinder *Finder) {
18+
auto RefField = fieldDecl(hasType(hasCanonicalType(referenceType(
19+
pointee(equalsBoundNode("type"))))))
20+
.bind("member");
21+
auto AssignLHS = memberExpr(
22+
hasObjectExpression(ignoringParenCasts(cxxThisExpr())), member(RefField));
23+
auto DerefOperand = expr(ignoringParenCasts(
24+
declRefExpr(to(parmVarDecl(equalsBoundNode("parm"))))));
25+
auto AssignRHS = expr(ignoringParenCasts(
26+
unaryOperator(hasOperatorName("*"), hasUnaryOperand(DerefOperand))));
27+
28+
auto BinaryOpAssign = binaryOperator(hasOperatorName("="), hasLHS(AssignLHS),
29+
hasRHS(AssignRHS));
30+
auto CXXOperatorCallAssign = cxxOperatorCallExpr(
31+
hasOverloadedOpera 9E88 torName("="), hasLHS(AssignLHS), hasRHS(AssignRHS));
32+
33+
auto SetBody =
34+
compoundStmt(statementCountIs(1),
35+
anyOf(has(BinaryOpAssign), has(CXXOperatorCallAssign)));
36+
auto BadSetFunction =
37+
cxxMethodDecl(
38+
parameterCountIs(1),
39+
hasParameter(
40+
0,
41+
parmVarDecl(hasType(hasCanonicalType(pointerType(pointee(qualType(
42+
hasCanonicalType(qualType().bind("type"))))))))
43+
.bind("parm")),
44+
hasBody(SetBody))
45+
.bind("bad-set-function");
46+
Finder->addMatcher(BadSetFunction, this);
47+
}
48+
49+
void MisleadingSetterOfReferenceCheck::check(
50+
const MatchFinder::MatchResult &Result) {
51+
const auto *Found = Result.Nodes.getNodeAs<CXXMethodDecl>("bad-set-function");
52+
const auto *Member = Result.Nodes.getNodeAs<FieldDecl>("member");
53+
54+
diag(Found->getBeginLoc(),
55+
"function '%0' can be mistakenly used in order to change the "
56+
"reference '%1' instead of the value of it; consider not using a "
57+
"pointer as argument")
58+
<< Found->getName() << Member->getName();
59+
}
60+
61+
} // namespace clang::tidy::bugprone

‎clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.h

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//===--- MisleadingSetterOfReferenceCheck.h - clang-tidy---------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISLEADINGSETTEROFREFERENCECHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISLEADINGSETTEROFREFERENCECHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::bugprone {
15+
16+
/// Emits a warning when a setter-like function that has a pointer parameter
17+
/// is used to set value of a field with reference type.
18+
///
19+
/// For the user-facing documentation see:
20+
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/throw-keyword-missing.html
21+
class MisleadingSetterOfReferenceCheck : public ClangTidyCheck {
22+
public:
23+
MisleadingSetterOfReferenceCheck(StringRef Name, ClangTidyContext *Context)
24+
: ClangTidyCheck(Name, Contex 10000 t) {}
25+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
26+
return LangOpts.CPlusPlus;
27+
}
28+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
29+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
30+
std::optional<TraversalKind> getCheckTraversalKind() const override {
31+
return TK_IgnoreUnlessSpelledInSource;
32+
}
33+
};
34+
35+
} // namespace clang::tidy::bugprone
36+
37+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISLEADINGSETTEROFREFERENCECHECK_H

‎clang-tools-extra/docs/ReleaseNotes.rst

+6
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,12 @@ New checks
124124
pointer and store it as class members without handle the copy and move
125125
constructors and the assignments.
126126

127+
- New :doc:`bugprone-misleading-setter-of-reference
128+
<clang-tidy/checks/bugprone/misleading-setter-of-reference>` check.
129+
130+
Finds setter-like member functions that take a pointer parameter and set a
131+
reference member of the same class with the pointed value.
132+
127133
- New :doc:`bugprone-unintended-char-ostream-output
128134
<clang-tidy/checks/bugprone/unintended-char-ostream-output>` check.
129135

‎clang-tools-extra/docs/clang-tidy/checks/bugprone/misleading-setter-of-reference.rst

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
.. title:: clang-tidy - bugprone-misleading-setter-of-reference
2+
3+
bugprone-misleading-setter-of-reference
4+
=======================================
5+
6+
Finds setter-like member functions that take a pointer parameter and set a
7+
reference member of the same class with the pointed value.
8+
9+
The check detects member functions that take a single pointer parameter,
10+
and contain a single expression statement that dereferences the parameter and
11+
assigns the result to a data member with a reference type.
12+
13+
The fact that a setter function takes a pointer might cause the belief that an
14+
internal reference (if it would be a pointer) is changed instead of the
15+
pointed-to (or referenced) value.
16+
17+
Example:
18+
19+
.. code-block:: c++
20+
21+
class MyClass {
22+
int &InternalRef; // non-const reference member
23+
public:
24+
MyClass(int &Value) : InternalRef(Value) {}
25+
26+
// Warning: This setter could lead to unintended behaviour.
27+
void setRef(int *Value) {
28+
InternalRef = *Value; // This assigns to the referenced value, not changing what InternalRef references.
29+
}
30+
};
31+
32+
int main() {
33+
int Value1 = 42;
34+
int Value2 = 100;
35+
MyClass X(Value1);
36+
37+
// This might look like it changes what InternalRef references to,
38+
// but it actually modifies Value1 to be 100.
39+
X.setRef(&Value2);
40+
}
41+
F438 42+
Possible fixes:
43+
- Change the parameter type of the "set" function to non-pointer type (for
44+
example, a const reference).
45+
- Change the type of the member variable to a pointer and in the "set"
46+
function assign a value to the pointer (without dereference).

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

+1
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ Clang-Tidy Checks
109109
:doc:`bugprone-lambda-function-name <bugprone/lambda-function-name>`,
110110
:doc:`bugprone-macro-parentheses <bugprone/macro-parentheses>`, "Yes"
111111
:doc:`bugprone-macro-repeated-side-effects <bugprone/macro-repeated-side-effects>`,
112+
:doc:`bugprone-misleading-setter-of-reference <bugprone/misleading-setter-of-reference>`,
112113
:doc:`bugprone-misplaced-operator-in-strlen-in-alloc <bugprone/misplaced-operator-in-strlen-in-alloc>`, "Yes"
113114
:doc:`bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone/misplaced-pointer-arithmetic-in-alloc>`, "Yes"
114115
:doc:`bugprone-misplaced-widening-cast <bugprone/misplaced-widening-cast>`,

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/misleading-setter-of-reference.cpp

+121
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// RUN: %check_clang_tidy %s bugprone-misleading-setter-of-reference %t
2+
3+
struct X {
4+
X &operator=(const X &) { return *this; }
5+
private:
6+
int &Mem;
7+
friend class Test1;
8+
};
9+
10+
class Test1 {
11+
X &MemX;
12+
int &MemI;
13+
protected:
14+
long &MemL;
15+
public:
16+
long &MemLPub;
17+
18+
Test1(X &MemX, int &MemI, long &MemL) : MemX(MemX), MemI(MemI), MemL(MemL), MemLPub(MemL) {}
19+
void setI(int *NewValue) {
20+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setI' can be mistakenly used in order to change the reference 'MemI' instead of the value of it
21+
MemI = *NewValue;
22+
}
23+
void setL(long *NewValue) {
24+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setL' can be mistakenly used in order to change the reference 'MemL' instead of the value of it
25+
MemL = *NewValue;
26+
}
27+
void setX(X *NewValue) {
28+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setX' can be mistakenly used in order to change the reference 'MemX' instead of the value of it
29+
MemX = *NewValue;
30+
}
31+
void set1(int *NewValue) {
32+
MemX.Mem = *NewValue;
33+
}
34+
void set2(int *NewValue) {
35+
MemL = static_cast<long>(*NewValue);
36+
}
37+
void set3(int *NewValue) {
38+
MemI = *NewValue;
39+
MemL = static_cast<long>(*NewValue);
40+
}
41+
void set4(long *NewValue, int) {
42+
MemL = *NewValue;
43+
}
44+
void setLPub(long *NewValue) {
45+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setLPub' can be mistakenly used in order to change the reference 'MemLPub' instead of the value of it
46+
MemLPub = *NewValue;
47+
}
48+
49+
private:
50+
void set5(long *NewValue) {
51+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'set5' can be mistakenly used in order to change the reference 'MemL' instead of the value of it
52+
MemL = *NewValue;
53+
}
54+
};
55+
56+
class Base {
57+
protected:
58+
int &MemI;
59+
};
60+
61+
class Derived : public Base {
62+
public:
63+
void setI(int *NewValue) {
64+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setI' can be mistakenly used in order to change the reference 'MemI' instead of the value of it
65+
MemI = *NewValue;
66+
}
67+
};
68+
69+
using UIntRef = unsigned int &;
70+
using UIntPtr = unsigned int *;
71+
using UInt = unsigned int;
72+
73+
class AliasTest {
74+
UIntRef Value1;
75+
UInt &Value2;
76+
unsigned int &Value3;
77+
public:
78+
void setValue1(UIntPtr NewValue) {
79+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setValue1' can be mistakenly used in order to change the reference 'Value1' instead of the value of it
80+
Value1 = *NewValue;
81+
}
82+
void setValue2(unsigned int *NewValue) {
83+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setValue2' can be mistakenly used in order to change the reference 'Value2' instead of the value of it
84+
Value2 = *NewValue;
85+
}
86+
void setValue3(UInt *NewValue) {
87+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setValue3' can be mistakenly used in order to change the reference 'Value3' instead of the value of it
88+
Value3 = *NewValue;
89+
}
90+
};
91+
92+
template <typename T>
93+
class TemplateTest {
94+
T &Mem;
95+
public:
96+
TemplateTest(T &V) : Mem{V} {}
97+
void setValue(T *NewValue) {
98+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setValue' can be mistakenly used in order to change the reference 'Mem' instead of the value of it
99+
Mem = *NewValue;
100+
}
101+
};
102+
103+
void f_TemplateTest(char *Value) {
104+
char CharValue;
105+
TemplateTest<char> TTChar{CharValue};
106+
TTChar.setValue(Value);
107+
}
108+
109+
template <typename T>
110+
class AddMember {
111+
protected:
112+
T &Value;
113+
};
114+
115+
class TemplateBaseTest : public AddMember<int> {
116+
public:
117+
void setValue(int *NewValue) {
118+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'setValue' can be mistakenly used in order to change the reference 'Value' instead of the value of it
119+
Value = *NewValue;
120+
}
121+
};

0 commit comments

Comments
 (0)
0