diff --git a/cpp/common/src/codingstandards/cpp/ast/ValueCategory.qll b/cpp/common/src/codingstandards/cpp/ast/ValueCategory.qll new file mode 100644 index 0000000000..803cfb3e7a --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/ast/ValueCategory.qll @@ -0,0 +1,128 @@ +import cpp + +/** + * Get an expression's value category as a ValueCategory object. + * + * Note that the standard cpp library exposes `is{_}ValueCategory` predicates, but they do not + * necessarily work as expected due to how CodeQL handles reference adjustment and binding, which + * this predicate attempts to handle. There are additional unhandled cases involving + * lvalue-to-rvalue conversions as well. + * + * In C++17, an expression of type "reference to T" is adjusted to type T as stated in [expr]/5. + * This is not a conversion, but in CodeQL this is handled by `ReferenceDereferenceExpr`, a type of + * `Conversion`. Similarly, the binding of references to values is described in [dcl.init.ref], + * which is not a conversion, but in CodeQL this is handled by `ReferenceToExpr`, another type of + * `Conversion`. + * + * Furthermore, the `Expr` predicate `hasLValueToRValueConversion()` uses a different dbscheme table + * than the `Conversion` table, and it is possible to have expressions such that + * `hasLValueToRValueConversion()` holds, but there is no corresponding `Conversion` entry. + * + * Therefore, the value categories of expressions before `ReferenceDereferenceExpr` and after + * `ReferenceToExpr` conversions are therefore essentially unspecified, and the `is{_}ValueCategory` + * predicate results should not be relied upon. And types that are missing a + * `LvalueToRValueConversion` will also return incorrect value categories. + * + * For example, in CodeQL 2.21.4: + * + * ```cpp + * int &i = ...; + * auto r = std::move(i); // 1.) i is a `prvalue(load)` in CodeQL, not an lvalue. + * // 2.) i has a `ReferenceDereferenceExpr` conversion of lvalue category + * // 3.) the conversion from 2 has a `ReferenceToExpr` conversion of prvalue + * // category. + * int i2 = ...; + * f(i2); // 1.) i2 is an lvalue. + * // 2.) i2 undergoes lvalue-to-rvalue conversion, but there is no corresponding `Conversion`. + * // 3.) i2 is treated at a prvalue by CodeQL, but `hasLValueToRValueConversion()` holds. + * + * int get_int(); + * auto r2 = std::move(get_int()); // 1.) get_int() itself is treated as a prvalue by CodeQL. + * // 2.) get_int() has a `TemporaryObjectExpr` conversion of lvalue + * // category. + * // 3.) the conversion from 2 has a `ReferenceToExpr` conversion + * // of prvalue category. + * std::string get_str(); + * auto r3 = std::move(get_str()); // 1.) get_str() is treated as a prvalue by CodeQL. + * // 2.) get_str() has a `TemporaryObjectExpr` conversion of xvalue + * // 3.) the conversion from 2 has a `ReferenceToExpr` conversion + * // of prvalue category. + * std::string &str_ref(); + * auto r3 = std::move(str_ref()); // 1.) str_ref() is treated as a prvalue by CodeQL. + * // 2.) str_ref() has a `ReferenceDereferenceExpr` conversion of + * // lvalue. + * // 3.) the conversion from 2 has a `ReferenceToExpr` conversion + * // of prvalue category. + * ``` + * + * As can be seen above, the value categories of expressions are correct between the + * `ReferenceDereferenceExpr` and `ReferenceToExpr`, but not necessarily before or after. + * + * We must also check for `hasLValueToRValueConversion()` and handle that appropriately. + */ +ValueCategory getValueCategory(Expr e) { + // If `e` is adjusted from a reference to a value (C++17 [expr]/5) then we want the value category + // of the expression after `ReferenceDereferenceExpr`. + if e.getConversion() instanceof ReferenceDereferenceExpr + then result = getValueCategory(e.getConversion()) + else ( + // If `hasLValueToRValueConversion()` holds, then ensure we have an lvalue category. + if e.hasLValueToRValueConversion() + then result.isLValue() + else + // Otherwise, get the value category from `is{_}ValueCategory` predicates as normal. + result = getDirectValueCategory(e) + ) +} + +/** + * Gets the value category of an expression using `is{_}ValueCategory` predicates, without looking + * through conversions. + */ +private ValueCategory getDirectValueCategory(Expr e) { + if e.isLValueCategory() + then result = LValue(e.getValueCategoryString()) + else + if e.isPRValueCategory() + then result = PRValue(e.getValueCategoryString()) + else + if e.isXValueCategory() + then result = XValue(e.getValueCategoryString()) + else none() +} + +newtype TValueCategory = + LValue(string descr) { + exists(Expr e | e.isLValueCategory() and descr = e.getValueCategoryString()) + } or + PRValue(string descr) { + exists(Expr e | e.isPRValueCategory() and descr = e.getValueCategoryString()) + } or + XValue(string descr) { + exists(Expr e | e.isXValueCategory() and descr = e.getValueCategoryString()) + } + +/** + * A value category, which can be an lvalue, prvalue, or xvalue. + * + * Note that prvalue has two possible forms: `prvalue` and `prvalue(load)`. + */ +class ValueCategory extends TValueCategory { + string description; + + ValueCategory() { + this = LValue(description) or this = PRValue(description) or this = XValue(description) + } + + predicate isLValue() { this instanceof LValue } + + predicate isPRValue() { this instanceof PRValue } + + predicate isXValue() { this instanceof XValue } + + predicate isRValue() { this instanceof PRValue or this instanceof XValue } + + predicate isGLValue() { this instanceof LValue or this instanceof XValue } + + string toString() { result = description } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions5.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions5.qll new file mode 100644 index 0000000000..8256a4ecc7 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Preconditions5.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Preconditions5Query = TStdMoveWithNonConstLvalueQuery() + +predicate isPreconditions5QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `stdMoveWithNonConstLvalue` query + Preconditions5Package::stdMoveWithNonConstLvalueQuery() and + queryId = + // `@id` for the `stdMoveWithNonConstLvalue` query + "cpp/misra/std-move-with-non-const-lvalue" and + ruleId = "RULE-28-6-1" and + category = "required" +} + +module Preconditions5Package { + Query stdMoveWithNonConstLvalueQuery() { + //autogenerate `Query` type + result = + // `Query` type for `stdMoveWithNonConstLvalue` query + TQueryCPP(TPreconditions5PackageQuery(TStdMoveWithNonConstLvalueQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 8771bbc79e..7210f36f91 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -80,6 +80,7 @@ import Pointers import Preconditions1 import Preconditions3 import Preconditions4 +import Preconditions5 import Preprocessor import Preprocessor2 import Representation @@ -182,6 +183,7 @@ newtype TCPPQuery = TPreconditions1PackageQuery(Preconditions1Query q) or TPreconditions3PackageQuery(Preconditions3Query q) or TPreconditions4PackageQuery(Preconditions4Query q) or + TPreconditions5PackageQuery(Preconditions5Query q) or TPreprocessorPackageQuery(PreprocessorQuery q) or TPreprocessor2PackageQuery(Preprocessor2Query q) or TRepresentationPackageQuery(RepresentationQuery q) or @@ -284,6 +286,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isPreconditions1QueryMetadata(query, queryId, ruleId, category) or isPreconditions3QueryMetadata(query, queryId, ruleId, category) or isPreconditions4QueryMetadata(query, queryId, ruleId, category) or + isPreconditions5QueryMetadata(query, queryId, ruleId, category) or isPreprocessorQueryMetadata(query, queryId, ruleId, category) or isPreprocessor2QueryMetadata(query, queryId, ruleId, category) or isRepresentationQueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/ValueCategoryTest.expected b/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/ValueCategoryTest.expected new file mode 100644 index 0000000000..4fe8acf995 --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/ValueCategoryTest.expected @@ -0,0 +1,48 @@ +| test.cpp:14:3:14:9 | call to get_val | prvalue | +| test.cpp:15:3:15:9 | call to get_ref | lvalue | +| test.cpp:16:3:16:10 | call to get_rref | xvalue | +| test.cpp:19:12:19:14 | val | lvalue | +| test.cpp:20:12:20:14 | val | lvalue | +| test.cpp:21:13:21:16 | call to move | xvalue | +| test.cpp:21:18:21:20 | val | lvalue | +| test.cpp:22:12:22:14 | val | lvalue | +| test.cpp:23:12:23:15 | call to move | xvalue | +| test.cpp:23:17:23:19 | val | lvalue | +| test.cpp:25:12:25:18 | call to get_val | prvalue | +| test.cpp:26:13:26:19 | call to get_val | prvalue | +| test.cpp:27:13:27:16 | call to move | xvalue | +| test.cpp:27:18:27:24 | call to get_val | prvalue | +| test.cpp:28:12:28:18 | call to get_val | prvalue | +| test.cpp:29:12:29:15 | call to move | xvalue | +| test.cpp:29:17:29:23 | call to get_val | prvalue | +| test.cpp:31:14:31:16 | val | lvalue | +| test.cpp:32:12:32:14 | ref | lvalue | +| test.cpp:33:12:33:14 | ref | lvalue | +| test.cpp:34:13:34:16 | call to move | xvalue | +| test.cpp:34:18:34:20 | ref | lvalue | +| test.cpp:35:12:35:14 | ref | lvalue | +| test.cpp:36:12:36:15 | call to move | xvalue | +| test.cpp:36:17:36:19 | ref | lvalue | +| test.cpp:38:12:38:18 | call to get_ref | lvalue | +| test.cpp:39:12:39:18 | call to get_ref | lvalue | +| test.cpp:40:13:40:16 | call to move | xvalue | +| test.cpp:40:18:40:24 | call to get_ref | lvalue | +| test.cpp:41:12:41:18 | call to get_ref | lvalue | +| test.cpp:42:12:42:15 | call to move | xvalue | +| test.cpp:42:17:42:23 | call to get_ref | lvalue | +| test.cpp:44:16:44:19 | call to move | xvalue | +| test.cpp:44:21:44:23 | val | lvalue | +| test.cpp:45:12:45:15 | rref | lvalue | +| test.cpp:46:12:46:15 | rref | lvalue | +| test.cpp:47:13:47:16 | call to move | xvalue | +| test.cpp:47:18:47:21 | rref | lvalue | +| test.cpp:48:12:48:15 | rref | lvalue | +| test.cpp:49:12:49:15 | call to move | xvalue | +| test.cpp:49:17:49:20 | rref | lvalue | +| test.cpp:51:12:51:19 | call to get_rref | lvalue | +| test.cpp:52:13:52:20 | call to get_rref | xvalue | +| test.cpp:53:13:53:16 | call to move | xvalue | +| test.cpp:53:18:53:25 | call to get_rref | xvalue | +| test.cpp:54:12:54:19 | call to get_rref | xvalue | +| test.cpp:55:12:55:15 | call to move | xvalue | +| test.cpp:55:17:55:24 | call to get_rref | xvalue | diff --git a/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/ValueCategoryTest.ql b/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/ValueCategoryTest.ql new file mode 100644 index 0000000000..224e9fd1c1 --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/ValueCategoryTest.ql @@ -0,0 +1,15 @@ +import cpp +import codingstandards.cpp.ast.ValueCategory + +predicate isRelevant(Expr e) { + e.(VariableAccess).getTarget().hasName(["val", "ref", "rref"]) + or + e.(FunctionCall).getTarget().hasName(["get_val", "get_ref", "get_rref", "move"]) +} + +from Expr e, ValueCategory cat +where + e.getEnclosingFunction().hasName("main") and + isRelevant(e) and + cat = getValueCategory(e) +select e, cat diff --git a/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/test.cpp b/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/test.cpp new file mode 100644 index 0000000000..449e11bd8d --- /dev/null +++ b/cpp/common/test/library/codingstandards/cpp/ast/ValueCategory/test.cpp @@ -0,0 +1,58 @@ +void take_val(int) {} +void take_ref(int &) {} +void take_rref(int &&) {} +template void take_uni(T &&) {} + +int get_val(); +int &get_ref(); +int &&get_rref(); + +template T &&move(T &t) { return static_cast(t); } +template T &&move(T &&t) { return static_cast(t); } + +int main() { + get_val(); + get_ref(); + get_rref(); + + int val = 42; + take_val(val); + take_ref(val); + take_rref(move(val)); + take_uni(val); + take_uni(move(val)); + + take_val(get_val()); + take_rref(get_val()); + take_rref(move(get_val())); + take_uni(get_val()); + take_uni(move(get_val())); + + int &ref = val; + take_val(ref); + take_ref(ref); + take_rref(move(ref)); + take_uni(ref); + take_uni(move(ref)); + + take_val(get_ref()); + take_ref(get_ref()); + take_rref(move(get_ref())); + take_uni(get_ref()); + take_uni(move(get_ref())); + + int &&rref = move(val); + take_val(rref); + take_ref(rref); + take_rref(move(rref)); + take_uni(rref); + take_uni(move(rref)); + + take_val(get_rref()); + take_rref(get_rref()); + take_rref(move(get_rref())); + take_uni(get_rref()); + take_uni(move(get_rref())); + + return 0; +} \ No newline at end of file diff --git a/cpp/misra/src/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.ql b/cpp/misra/src/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.ql new file mode 100644 index 0000000000..05d0dd1caa --- /dev/null +++ b/cpp/misra/src/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.ql @@ -0,0 +1,56 @@ +/** + * @id cpp/misra/std-move-with-non-const-lvalue + * @name RULE-28-6-1: The argument to std::move shall be a non-const lvalue + * @description Calling std::move on a const lvalue will not result in a move, and calling std::move + * on an rvalue is redundant. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-28-6-1 + * scope/single-translation-unit + * correctness + * maintainability + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.standardlibrary.Utility +import codingstandards.cpp.ast.ValueCategory + +predicate resolvesToConstOrConstRef(Type t) { + t.isConst() + or + resolvesToConstOrConstRef(t.(ReferenceType).getBaseType()) + or + resolvesToConstOrConstRef(t.(TypedefType).getBaseType()) + or + resolvesToConstOrConstRef(t.(Decltype).getBaseType()) +} + +predicate isConstLvalue(Expr arg) { + getValueCategory(arg).isLValue() and + resolvesToConstOrConstRef(arg.getType()) +} + +Type typeOfArgument(Expr e) { + // An xvalue may be a constructor, which has no return type. However, these xvalues act as values + // of the constructed type. + if e instanceof ConstructorCall + then result = e.(ConstructorCall).getTargetType() + else result = e.getType() +} + +from StdMoveCall call, Expr arg, string description +where + arg = call.getArgument(0) and + ( + isConstLvalue(arg) and + description = "const " + getValueCategory(arg).toString() + or + getValueCategory(arg).isRValue() and + description = getValueCategory(arg).toString() + ) +select call, + "Call to 'std::move' with " + description + " argument of type '" + typeOfArgument(arg).toString() + + "'." diff --git a/cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.expected b/cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.expected new file mode 100644 index 0000000000..da757ab979 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.expected @@ -0,0 +1,47 @@ +| test.cpp:62:3:62:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const string &'. | +| test.cpp:63:3:63:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const int &'. | +| test.cpp:64:3:64:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const C &'. | +| test.cpp:66:8:66:16 | call to move | Call to 'std::move' with const lvalue argument of type 'const string &'. | +| test.cpp:67:8:67:16 | call to move | Call to 'std::move' with const lvalue argument of type 'const int &'. | +| test.cpp:68:8:68:16 | call to move | Call to 'std::move' with const lvalue argument of type 'const C &'. | +| test.cpp:75:3:75:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const string'. | +| test.cpp:76:3:76:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const int'. | +| test.cpp:77:3:77:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const C'. | +| test.cpp:93:3:93:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const string &&'. | +| test.cpp:94:3:94:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const int &&'. | +| test.cpp:95:3:95:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const C &&'. | +| test.cpp:98:3:98:11 | call to move | Call to 'std::move' with prvalue argument of type 'basic_string, allocator>'. | +| test.cpp:99:3:99:11 | call to move | Call to 'std::move' with prvalue argument of type 'int'. | +| test.cpp:100:3:100:11 | call to move | Call to 'std::move' with prvalue argument of type 'C'. | +| test.cpp:102:8:102:16 | call to move | Call to 'std::move' with prvalue argument of type 'basic_string, allocator>'. | +| test.cpp:103:8:103:16 | call to move | Call to 'std::move' with prvalue argument of type 'int'. | +| test.cpp:104:8:104:16 | call to move | Call to 'std::move' with prvalue argument of type 'C'. | +| test.cpp:106:3:106:11 | call to move | Call to 'std::move' with prvalue argument of type 'basic_string, allocator>'. | +| test.cpp:107:3:107:11 | call to move | Call to 'std::move' with prvalue argument of type 'int'. | +| test.cpp:108:3:108:11 | call to move | Call to 'std::move' with prvalue argument of type 'string'. | +| test.cpp:109:3:109:11 | call to move | Call to 'std::move' with prvalue argument of type 'int'. | +| test.cpp:110:3:110:11 | call to move | Call to 'std::move' with prvalue argument of type 'C'. | +| test.cpp:118:3:118:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const string &'. | +| test.cpp:119:3:119:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const int &'. | +| test.cpp:120:3:120:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const C &'. | +| test.cpp:123:3:123:11 | call to move | Call to 'std::move' with xvalue argument of type 'string &&'. | +| test.cpp:124:3:124:11 | call to move | Call to 'std::move' with xvalue argument of type 'int &&'. | +| test.cpp:125:3:125:11 | call to move | Call to 'std::move' with xvalue argument of type 'C &&'. | +| test.cpp:127:3:127:11 | call to move | Call to 'std::move' with xvalue argument of type 'const string &&'. | +| test.cpp:128:3:128:11 | call to move | Call to 'std::move' with xvalue argument of type 'const int &&'. | +| test.cpp:129:3:129:11 | call to move | Call to 'std::move' with xvalue argument of type 'const C &&'. | +| test.cpp:139:5:139:13 | call to move | Call to 'std::move' with const lvalue argument of type 'const string'. | +| test.cpp:145:56:145:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:147:56:147:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:153:56:153:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:159:56:159:64 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:164:49:164:57 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:168:49:168:57 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:172:49:172:57 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:177:48:177:56 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator>'. | +| test.cpp:181:48:181:56 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:185:48:185:56 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &&'. | +| test.cpp:190:55:190:63 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &&'. | +| test.cpp:194:55:194:63 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &'. | +| test.cpp:198:55:198:63 | call to move | Call to 'std::move' with const lvalue argument of type 'const basic_string, allocator> &&'. | +| test.cpp:238:3:238:11 | call to move | Call to 'std::move' with const lvalue argument of type 'const iterator &'. | diff --git a/cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.qlref b/cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.qlref new file mode 100644 index 0000000000..c1906c448e --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-6-1/StdMoveWithNonConstLvalue.qlref @@ -0,0 +1 @@ +rules/RULE-28-6-1/StdMoveWithNonConstLvalue.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-28-6-1/test.cpp b/cpp/misra/test/rules/RULE-28-6-1/test.cpp new file mode 100644 index 0000000000..d94b553543 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-6-1/test.cpp @@ -0,0 +1,241 @@ +#include +#include + +class C { + int x; +}; + +template void func(T &¶m) {} + +std::string get_str(); +int get_int(); +C get_cls(); + +std::string &get_str_ref(); +int &get_int_ref(); +C &get_cls_ref(); + +const std::string &get_str_cref(); +const int &get_int_cref(); +const C &get_cls_cref(); + +std::string &&get_str_rref(); +int &&get_int_rref(); +C &&get_cls_rref(); + +const std::string &&get_str_crref(); +const int &&get_int_crref(); +const C &&get_cls_crref(); + +void f2() { + // non-const lvalues + std::string l1{"hello"}; + int il1{0}; + C cl1{}; + + std::move(l1); // COMPLIANT + std::move(il1); // COMPLIANT + std::move(cl1); // COMPLIANT + + func(std::move(l1)); // COMPLIANT + func(std::move(il1)); // COMPLIANT + func(std::move(cl1)); // COMPLIANT + + // non-const lvalue references + std::string &l2 = l1; + int &il2 = il1; + C &cl2 = cl1; + + std::move(l2); // COMPLIANT + std::move(il2); // COMPLIANT + std::move(cl2); // COMPLIANT + + func(std::move(l2)); // COMPLIANT + func(std::move(il2)); // COMPLIANT + func(std::move(cl2)); // COMPLIANT + + // const lvalue refs + std::string const &l3 = l1; + int const &il3 = il1; + C const &cl3 = cl1; + + std::move(l3); // NON_COMPLIANT + std::move(il3); // NON_COMPLIANT + std::move(cl3); // NON_COMPLIANT + + func(std::move(l3)); // NON_COMPLIANT + func(std::move(il3)); // NON_COMPLIANT + func(std::move(cl3)); // NON_COMPLIANT + + // const lvalues + std::string const l4{"hello"}; + int const il4{0}; + C const cl4{}; + + std::move(l4); // NON_COMPLIANT + std::move(il4); // NON_COMPLIANT + std::move(cl4); // NON_COMPLIANT + + // non-const lvalues of rvalue reference type + std::string &&l5 = std::string{"hello"}; + int &&il5 = int{0}; + C &&cl5 = C{}; + + std::move(l5); // COMPLIANT + std::move(il5); // COMPLIANT + std::move(cl5); // COMPLIANT + + // const lvalues of rvalue reference type + const std::string &&l6 = std::string{"hello"}; + const int &&il6 = int{0}; + const C &&cl6 = C{}; + + std::move(l6); // NON_COMPLIANT + std::move(il6); // NON_COMPLIANT + std::move(cl6); // NON_COMPLIANT + + // xvalues + std::move(std::string("hello")); // NON_COMPLIANT + std::move(int(1)); // NON_COMPLIANT + std::move(C()); // NON_COMPLIANT + + func(std::move(std::string("hello"))); // NON_COMPLIANT + func(std::move(int(1))); // NON_COMPLIANT + func(std::move(C())); // NON_COMPLIANT + + std::move(l1 + "!"); // NON_COMPLIANT + std::move(il1 + 1); // NON_COMPLIANT + std::move(get_str()); // NON_COMPLIANT + std::move(get_int()); // NON_COMPLIANT + std::move(get_cls()); // NON_COMPLIANT + + // Function calls returning references + std::move(get_str_ref()); // COMPLIANT + std::move(get_int_ref()); // COMPLIANT + std::move(get_cls_ref()); // COMPLIANT + + // Function calls returning const references + std::move(get_str_cref()); // NON_COMPLIANT + std::move(get_int_cref()); // NON_COMPLIANT + std::move(get_cls_cref()); // NON_COMPLIANT + + // Function calls returning rvalue references + std::move(get_str_rref()); // NON_COMPLIANT + std::move(get_int_rref()); // NON_COMPLIANT + std::move(get_cls_rref()); // NON_COMPLIANT + + std::move(get_str_crref()); // NON_COMPLIANT + std::move(get_int_crref()); // NON_COMPLIANT + std::move(get_cls_crref()); // NON_COMPLIANT +} + +class TestClass { +public: + std::string m1; + std::string const m2{"constant"}; + + void test_member_variables() { + std::move(m1); // COMPLIANT - non-const member + std::move(m2); // NON_COMPLIANT - const member + } +}; + +// Moving values that are always const: +// NON_COMPLIANT +template void cref_tpl1(const T ¶m) { std::move(param); } +// NON_COMPLIANT +template void cref_tpl2(const T ¶m) { std::move(param); } +// This is actually compliant, because the provided type `T` is a reference and +// the references collapse in a way that drops the const qualifier. +// COMPLIANT +template void cref_tpl3(const T ¶m) { std::move(param); } +// NON_COMPLIANT +template void cref_tpl4(const T ¶m) { std::move(param); } +// This is actually compliant, because the provided type `T` is a reference and +// the references collapse in a way that drops the const qualifier. +// COMPLIANT +template void cref_tpl5(const T ¶m) { std::move(param); } +// NON_COMPLIANT +template void cref_tpl6(const T ¶m) { std::move(param); } + +// T=std::string -- COMPLIANT +template void ref_tpl1(T ¶m) { std::move(param); } +// T=const std::string -- NON_COMPLIANT, move const lvalue ref +template void ref_tpl2(T ¶m) { std::move(param); } +// T=std::string& -- COMPLIANT +template void ref_tpl3(T ¶m) { std::move(param); } +// T=const std::string& -- NON_COMPLIANT, move const lvalue ref +template void ref_tpl4(T ¶m) { std::move(param); } +// T=std::string&& -- COMPLIANT +template void ref_tpl5(T ¶m) { std::move(param); } +// T=const std::string&& -- NON_COMPLIANT, move const rvalue ref +template void ref_tpl6(T ¶m) { std::move(param); } + +// T=std::string -- COMPLIANT +template void val_tpl1(T param) { std::move(param); } +// T=const std::string -- NON_COMPLIANT, move const lvalue +template void val_tpl2(T param) { std::move(param); } +// T=std::string& -- COMPLIANT +template void val_tpl3(T param) { std::move(param); } +// T=const std::string& -- NON_COMPLIANT -- move const lvalue ref +template void val_tpl4(T param) { std::move(param); } +// T=std::string&& -- COMPLIANT +template void val_tpl5(T param) { std::move(param); } +// T=const std::string&& -- NON_COMPLIANT -- move const rvalue ref +template void val_tpl6(T param) { std::move(param); } + +// T=std::string -- COMPLIANT +template void univ_ref_tpl1(T &¶m) { std::move(param); } +// T=const std::string -- NON_COMPLIANT -- moving const rvalue ref +template void univ_ref_tpl2(T &¶m) { std::move(param); } +// T=std::string& -- COMPLIANT +template void univ_ref_tpl3(T &¶m) { std::move(param); } +// T=const std::string& -- NON_COMPLIANT -- moving const lvalue ref +template void univ_ref_tpl4(T &¶m) { std::move(param); } +// T=std::string&& -- COMPLIANT +template void univ_ref_tpl5(T &¶m) { std::move(param); } +// T=const std::string&& -- NON_COMPLIANT +template void univ_ref_tpl6(T &¶m) { std::move(param); } + +void f3() { + std::string l1{"test"}; + const std::string l2{"test"}; + std::string &l3 = l1; + const std::string &l4 = l1; + + val_tpl1(l1); + val_tpl2(l2); + val_tpl3(l3); + val_tpl4(l4); + val_tpl5(""); + val_tpl6(""); + ref_tpl1(l1); + ref_tpl2(l2); + ref_tpl3(l3); + ref_tpl4(l4); + ref_tpl5(l1); + ref_tpl6(l1); + cref_tpl1(l1); + cref_tpl2(l2); + cref_tpl3(l3); + cref_tpl4(l4); + cref_tpl5(l1); + cref_tpl6(l1); + univ_ref_tpl1(""); + univ_ref_tpl2(""); + univ_ref_tpl3(l3); + univ_ref_tpl4(l4); + univ_ref_tpl5(""); + univ_ref_tpl6(""); +} + +#include +#include + +void algorithm() { + std::vector v; + const std::vector::iterator &it = v.begin(); + std::move(it); // NON_COMPLIANT -- from + std::move::iterator &>( + it, it, std::back_inserter(v)); // COMPLIANT -- from +} \ No newline at end of file diff --git a/rule_packages/cpp/Preconditions5.json b/rule_packages/cpp/Preconditions5.json new file mode 100644 index 0000000000..5a758c2fb0 --- /dev/null +++ b/rule_packages/cpp/Preconditions5.json @@ -0,0 +1,26 @@ +{ + "MISRA-C++-2023": { + "RULE-28-6-1": { + "properties": { + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "Calling std::move on a const lvalue will not result in a move, and calling std::move on an rvalue is redundant.", + "kind": "problem", + "name": "The argument to std::move shall be a non-const lvalue", + "precision": "very-high", + "severity": "error", + "short_name": "StdMoveWithNonConstLvalue", + "tags": [ + "scope/single-translation-unit", + "correctness", + "maintainability" + ] + } + ], + "title": "The argument to std::move shall be a non-const lvalue" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 7abe0599a4..4b542d356a 100644 --- a/rules.csv +++ b/rules.csv @@ -996,7 +996,7 @@ cpp,MISRA-C++-2023,RULE-25-5-2,Yes,Mandatory,Decidable,Single Translation Unit," 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, 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, cpp,MISRA-C++-2023,RULE-28-3-1,Yes,Required,Undecidable,System,Predicates shall not have persistent side effects,A25-1-1,SideEffects6,Easy, -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, +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, 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, 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, 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",,DeadCode11,Easy,