Skip to content

Commit 9c70506

Browse files
MichaelRFairhurstMichaelRFairhurst
committed
Implement preconditions rule 28-6-1, std::move of rvalue or const lvalue.
1 parent d20e6a5 commit 9c70506

File tree

9 files changed

+318
-1
lines changed

9 files changed

+318
-1
lines changed

‎cpp/common/src/codingstandards/cpp/ast/ValueCategory.qll‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import cpp
2+
3+
/**
4+
* Get an expression's value category as a ValueCategory object.
5+
*
6+
* Note that the standard cpp library exposes `is{_}ValueCategory` predicates, but they do not
7+
* correctly work with conversions. This function is intended to give the correct answer in the
8+
* presence of conversions such as lvalue-to-rvalue conversion.
9+
*/
10+
ValueCategory getValueCategory(Expr e) {
11+
not exists(e.getConversion()) and
12+
result = getDirectValueCategory(e)
13+
or
14+
if e.getConversion() instanceof ReferenceToExpr
15+
then result = getDirectValueCategory(e)
16+
else result = getDirectValueCategory(e.getConversion())
17+
}
18+
19+
private ValueCategory getDirectValueCategory(Expr e) {
20+
if e.isLValueCategory()
21+
then result = LValue(e.getValueCategoryString())
22+
else
23+
if e.isPRValueCategory()
24+
then result = PRValue(e.getValueCategoryString())
25+
else
26+
if e.isXValueCategory()
27+
then result = XValue(e.getValueCategoryString())
28+
else none()
29+
}
30+
31+
newtype TValueCategory =
32+
LValue(string descr) {
33+
exists(Expr e | e.isLValueCategory() and descr = e.getValueCategoryString())
34+
} or
35+
PRValue(string descr) {
36+
exists(Expr e | e.isPRValueCategory() and descr = e.getValueCategoryString())
37+
} or
38+
XValue(string descr) {
39+
exists(Expr e | e.isXValueCategory() and descr = e.getValueCategoryString())
40+
}
41+
42+
class ValueCategory extends TValueCategory {
43+
string description;
44+
45+
ValueCategory() {
46+
this = LValue(description) or this = PRValue(description) or this = XValue(description)
47+
}
48+
49+
predicate isLValue() { this instanceof LValue }
50+
51+
predicate isPRValue() { this instanceof PRValue }
52+
53+
predicate isXValue() { this instanceof XValue }
54+
55+
predicate isRValue() { this instanceof PRValue or this instanceof XValue }
56+
57+
predicate isGlvalue() { this instanceof LValue or this instanceof XValue }
58+
59+
string toString() { result = description }
60+
}

‎cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions5.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype Preconditions5Query = TStdMoveWithNonConstLvalueQuery()
7+
8+
predicate isPreconditions5QueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `stdMoveWithNonConstLvalue` query
11+
Preconditions5Package::stdMoveWithNonConstLvalueQuery() and
12+
queryId =
13+
// `@id` for the `stdMoveWithNonConstLvalue` query
14+
"cpp/misra/std-move-with-non-const-lvalue" and
15+
ruleId = "RULE-28-6-1" and
16+
category = "required"
17+
}
18+
19+
module Preconditions5Package {
20+
Query stdMoveWithNonConstLvalueQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `stdMoveWithNonConstLvalue` query
24+
TQueryCPP(TPreconditions5PackageQuery(TStdMoveWithNonConstLvalueQuery()))
25+
}
26+
}

‎cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import Operators
4444
import OrderOfEvaluation
4545
import OutOfBounds
4646
import Pointers
47+
import Preconditions5
4748
import Representation
4849
import Scope
4950
import SideEffects1
@@ -103,6 +104,7 @@ newtype TCPPQuery =
103104
TOrderOfEvaluationPackageQuery(OrderOfEvaluationQuery q) or
104105
TOutOfBoundsPackageQuery(OutOfBoundsQuery q) or
105106
TPointersPackageQuery(PointersQuery q) or
107+
TPreconditions5PackageQuery(Preconditions5Query q) or
106108
TRepresentationPackageQuery(RepresentationQuery q) or
107109
TScopePackageQuery(ScopeQuery q) or
108110
TSideEffects1PackageQuery(SideEffects1Query q) or
@@ -162,6 +164,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
162164
isOrderOfEvaluationQueryMetadata(query, queryId, ruleId, category) or
163165
isOutOfBoundsQueryMetadata(query, queryId, ruleId, category) or
164166
isPointersQueryMetadata(query, queryId, ruleId, category) or
167+
isPreconditions5QueryMetadata(query, queryId, ruleId, category) or
165168
isRepresentationQueryMetadata(query, queryId, ruleId, category) or
166169
isScopeQueryMetadata(query, queryId, ruleId, category) or
167170
isSideEffects1QueryMetadata(query, queryId, ruleId, category) or

‎cpp/misra/src/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.ql‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @id cpp/misra/std-move-with-non-const-lvalue
3+
* @name RULE-28-6-1: The argument to std::move shall be a non-const lvalue
4+
* @description Calling std::move on a const lvalue will not result in a move, and calling std::move
5+
* on an rvalue is redundant.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-28-6-1
10+
* scope/single-translation-unit
11+
* correctness
12+
* maintainability
13+
* external/misra/enforcement/decidable
14+
* external/misra/obligation/required
15+
*/
16+
17+
import cpp
18+
import codingstandards.cpp.standardlibrary.Utility
19+
import codingstandards.cpp.ast.ValueCategory
20+
import codingstandards.cpp.types.Resolve
21+
import codingstandards.cpp.types.Specifiers
22+
23+
predicate isConstLvalue(Expr arg) {
24+
getValueCategory(arg).isLValue() and
25+
arg.getType() instanceof ResolvesTo<RawConstType>::ExactlyOrRef
26+
}
27+
28+
Type typeOfArgument(Expr e) {
29+
// An xvalue may be a constructor, which has no return type. However, these xvalues as act values
30+
// of the constructed type.
31+
if e instanceof ConstructorCall
32+
then result = e.(ConstructorCall).getTargetType()
33+
else result = e.getType()
34+
}
35+
36+
from StdMoveCall call, Expr arg, string description
37+
where
38+
arg = call.getArgument(0) and
39+
(
40+
isConstLvalue(arg) and
41+
description = "const " + getValueCategory(arg).toString()
42+
or
43+
getValueCategory(arg).isRValue() and
44+
description = getValueCategory(arg).toString()
45+
)
46+
select call,
47+
"Call to 'std::move' with " + description + " argument of type '" + typeOfArgument(arg).toString()
48+
+ "'."

‎cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.expected‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
| test.cpp:18:8:18:16 | call to move | Call to 'std::move' with const lvalue argument of type 'const string &'. |
2+
| test.cpp:19:8:19:16 | call to move | Call to 'std::move' with const lvalue argument of type 'const string'. |
3+
| test.cpp:21:8:21:16 | call to move | Call to 'std::move' with const lvalue argument of type 'const string &&'. |
4+
| test.cpp:23:8:23:16 | call to move | Call to 'std::move' with xvalue argument of type 'basic_string<char, char_traits<char>, allocator<char>>'. |
5+
| test.cpp:24:8:24:16 | call to move | Call to 'std::move' with xvalue argument of type 'basic_string<char, char_traits<char>, allocator<char>>'. |
6+
| test.cpp:25:8:25:16 | call to move | Call to 'std::move' with xvalue argument of type 'string'. |
7+
| test.cpp:35:5:35:13 | call to move | Call to 'std::move' with const lvalue argument of type 'const string'. |
8+
| test.cpp:40:56:40:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
9+
| test.cpp:42:56:42:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
10+
| test.cpp:46:56:46:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
11+
| test.cpp:50:56:50:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
12+
| test.cpp:55:49:55:57 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
13+
| test.cpp:59:49:59:57 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
14+
| test.cpp:63:49:63:57 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
15+
| test.cpp:68:48:68:56 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>>'. |
16+
| test.cpp:72:48:72:56 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
17+
| test.cpp:76:48:76:56 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &&'. |
18+
| test.cpp:81:55:81:63 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &&'. |
19+
| test.cpp:85:55:85:63 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &'. |
20+
| test.cpp:89:55:89:63 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string<char, char_traits<char>, allocator<char>> &&'. |
21+
| test.cpp:129:3:129:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const iterator &'. |

‎cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/RULE-28-6-1/StdMoveWithNonConstLvalue.ql

‎cpp/misra/test/rules/RULE-28-6-1/test.cpp‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
#include <string>
2+
#include <utility>
3+
4+
template <typename T> T func(T param);
5+
6+
std::string f2();
7+
8+
void f3() {
9+
std::string l1{"hello"};
10+
std::string &l2 = l1;
11+
std::string const &l3 = l1;
12+
std::string const l4{"hello"};
13+
std::string &&l5 = std::string{"hello"};
14+
const std::string &&l6 = std::string{"hello"};
15+
16+
func(std::move(l1)); // COMPLIANT - non-const lvalue
17+
func(std::move(l2)); // COMPLIANT - non-const lvalue
18+
func(std::move(l3)); // NON_COMPLIANT - const lvalue
19+
func(std::move(l4)); // NON_COMPLIANT - const lvalue
20+
func(std::move(l5)); // COMPLIANT - non-const glvalue
21+
func(std::move(l6)); // NON_COMPLIANT - const lvalue
22+
23+
func(std::move(std::string("hello"))); // NON_COMPLIANT - rvalue argument
24+
func(std::move(l1 + "!")); // NON_COMPLIANT - temporary rvalue
25+
func(std::move(f2())); // NON_COMPLIANT - rvalue from function call
26+
}
27+
28+
class TestClass {
29+
public:
30+
std::string m1;
31+
std::string const m2{"constant"};
32+
33+
void test_member_variables() {
34+
std::move(m1); // COMPLIANT - non-const member
35+
std::move(m2); // NON_COMPLIANT - const member
36+
}
37+
};
38+
39+
// NON_COMPLIANT -- always moving const value
40+
template <typename T> void cref_tpl1(const T &param) { std::move(param); }
41+
// NON_COMPLIANT -- always moving const value
42+
template <typename T> void cref_tpl2(const T &param) { std::move(param); }
43+
// NON_COMPLIANT -- always moving const value
44+
template <typename T> void cref_tpl3(const T &param) { std::move(param); }
45+
// NON_COMPLIANT -- always moving const value
46+
template <typename T> void cref_tpl4(const T &param) { std::move(param); }
47+
// NON_COMPLIANT -- always moving const value
48+
template <typename T> void cref_tpl5(const T &param) { std::move(param); }
49+
// NON_COMPLIANT -- always moving const value
50+
template <typename T> void cref_tpl6(const T &param) { std::move(param); }
51+
52+
// T=std::string -- COMPLIANT
53+
template <typename T> void ref_tpl1(T &param) { std::move(param); }
54+
// T=const std::string -- NON_COMPLIANT, move const lvalue ref
55+
template <typename T> void ref_tpl2(T &param) { std::move(param); }
56+
// T=std::string& -- COMPLIANT
57+
template <typename T> void ref_tpl3(T &param) { std::move(param); }
58+
// T=const std::string& -- NON_COMPLIANT, move const lvalue ref
59+
template <typename T> void ref_tpl4(T &param) { std::move(param); }
60+
// T=std::string&& -- COMPLIANT
61+
template <typename T> void ref_tpl5(T &param) { std::move(param); }
62+
// T=const std::string&& -- NON_COMPLIANT, move const rvalue ref
63+
template <typename T> void ref_tpl6(T &param) { std::move(param); }
64+
65+
// T=std::string -- COMPLIANT
66+
template <typename T> void val_tpl1(T param) { std::move(param); }
67+
// T=const std::string -- NON_COMPLIANT, move const lvalue
68+
template <typename T> void val_tpl2(T param) { std::move(param); }
69+
// T=std::string& -- COMPLIANT
70+
template <typename T> void val_tpl3(T param) { std::move(param); }
71+
// T=const std::string& -- NON_COMPLIANT -- move const lvalue ref
72+
template <typename T> void val_tpl4(T param) { std::move(param); }
73+
// T=std::string&& -- COMPLIANT
74+
template <typename T> void val_tpl5(T param) { std::move(param); }
75+
// T=const std::string&& -- NON_COMPLIANT -- move const rvalue ref
76+
template <typename T> void val_tpl6(T param) { std::move(param); }
77+
78+
// T=std::string -- COMPLIANT
79+
template <typename T> void univ_ref_tpl1(T &&param) { std::move(param); }
80+
// T=const std::string -- NON_COMPLIANT -- moving const rvalue ref
81+
template <typename T> void univ_ref_tpl2(T &&param) { std::move(param); }
82+
// T=std::string& -- COMPLIANT
83+
template <typename T> void univ_ref_tpl3(T &&param) { std::move(param); }
84+
// T=const std::string& -- NON_COMPLIANT -- moving const lvalue ref
85+
template <typename T> void univ_ref_tpl4(T &&param) { std::move(param); }
86+
// T=std::string&& -- COMPLIANT
87+
template <typename T> void univ_ref_tpl5(T &&param) { std::move(param); }
88+
// T=const std::string&& -- NON_COMPLIANT
89+
template <typename T> void univ_ref_tpl6(T &&param) { std::move(param); }
90+
91+
void f4() {
92+
std::string l1{"test"};
93+
const std::string l2{"test"};
94+
std::string &l3 = l1;
95+
const std::string &l4 = l1;
96+
97+
val_tpl1<std::string>(l1);
98+
val_tpl2<const std::string>(l2);
99+
val_tpl3<std::string &>(l3);
100+
val_tpl4<const std::string &>(l4);
101+
val_tpl5<std::string &&>("");
102+
val_tpl6<const std::string &&>("");
103+
ref_tpl1<std::string>(l1);
104+
ref_tpl2<const std::string>(l2);
105+
ref_tpl3<std::string &>(l3);
106+
ref_tpl4<const std::string &>(l4);
107+
ref_tpl5<std::string &&>(l1);
108+
ref_tpl6<const std::string &&>(l1);
109+
cref_tpl1<std::string>(l1);
110+
cref_tpl2<const std::string>(l2);
111+
cref_tpl3<std::string &>(l3);
112+
cref_tpl4<const std::string &>(l4);
113+
cref_tpl5<std::string &&>(l1);
114+
cref_tpl6<const std::string &&>(l1);
115+
univ_ref_tpl1<std::string>("");
116+
univ_ref_tpl2<const std::string>("");
117+
univ_ref_tpl3<std::string &>(l3);
118+
univ_ref_tpl4<const std::string &>(l4);
119+
univ_ref_tpl5<std::string &&>("");
120+
univ_ref_tpl6<const std::string &&>("");
121+
}
122+
123+
#include <algorithm>
124+
#include <vector>
125+
126+
void algorithm() {
127+
std::vector<int> v;
128+
const std::vector<int>::iterator &it = v.begin();
129+
std::move(it); // NON_COMPLIANT -- from <utility>
130+
std::move<const std::vector<int>::iterator &>(
131+
it, it, std::back_inserter(v)); // COMPLIANT -- from <algorithm>
132+
}

‎rule_packages/cpp/Preconditions5.json‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"MISRA-C++-2023": {
3+
"RULE-28-6-1": {
4+
"properties": {
5+
"enforcement": "decidable",
6+
"obligation": "required"
7+
},
8+
"queries": [
9+
{
10+
"description": "Calling std::move on a const lvalue will not result in a move, and calling std::move on an rvalue is redundant.",
11+
"kind": "problem",
12+
"name": "The argument to std::move shall be a non-const lvalue",
13+
"precision": "very-high",
14+
"severity": "error",
15+
"short_name": "StdMoveWithNonConstLvalue",
16+
"tags": [
17+
"scope/single-translation-unit",
18+
"correctness",
19+
"maintainability"
20+
]
21+
}
22+
],
23+
"title": "The argument to std::move shall be a non-const lvalue"
24+
}
25+
}
26+
}

‎rules.csv‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -996,7 +996,7 @@ cpp,MISRA-C++-2023,RULE-25-5-2,Yes,Mandatory,Decidable,Single Translation Unit,"
996996
cpp,MISRA-C++-2023,RULE-25-5-3,Yes,Mandatory,Undecidable,System,"The pointer returned by the C++ Standard Library functions asctime, ctime, gmtime, localtime, localeconv, getenv, setlocale or strerror must not be used following a subsequent call to the same function",RULE-21-20,ImportMisra23,Import,
997997
cpp,MISRA-C++-2023,RULE-26-3-1,Yes,Advisory,Decidable,Single Translation Unit,std::vector should not be specialized with bool,A18-1-2,ImportMisra23,Import,
998998
cpp,MISRA-C++-2023,RULE-28-3-1,Yes,Required,Undecidable,System,Predicates shall not have persistent side effects,A25-1-1,SideEffects3,Easy,
999-
cpp,MISRA-C++-2023,RULE-28-6-1,Yes,Required,Decidable,Single Translation Unit,The argument to std::move shall be a non-const lvalue,A18-9-3,Preconditions,Easy,
999+
cpp,MISRA-C++-2023,RULE-28-6-1,Yes,Required,Decidable,Single Translation Unit,The argument to std::move shall be a non-const lvalue,A18-9-3,Preconditions5,Easy,
10001000
cpp,MISRA-C++-2023,RULE-28-6-2,Yes,Required,Decidable,Single Translation Unit,Forwarding references and std::forward shall be used together,A18-9-2,ImportMisra23,Import,
10011001
cpp,MISRA-C++-2023,RULE-28-6-3,Yes,Required,Decidable,Single Translation Unit,An object shall not be used while in a potentially moved-from state,A12-8-3,ImportMisra23,Import,
10021002
cpp,MISRA-C++-2023,RULE-28-6-4,Yes,Required,Decidable,Single Translation Unit,"The result of std::remove, std::remove_if, std::unique and empty shall be used",,DeadCode2,Easy,

0 commit comments

Comments
 (0)