Skip to content

Commit c45b003

Browse files
committed
Fix RULE-7-0-1 false positives for bool-to-reference bindings
`NoConversionFromBool.ql` excludes conversions whose destination type, after `stripTopLevelSpecifiers()`, is `bool`. `ReferenceType` does not override `stripTopLevelSpecifiers()`, so a `bool` value bound to a `bool&`/`const bool&` parameter is not stripped down to `BoolType` and the rule fires even though no actual type conversion happens (only a reference binding to the same type). This is confirmed against eclipse-score/communication, where nearly all (46 of 51) open RULE-7-0-1 alerts are of this shape: - `bool&`/`const bool&` out-parameters and variadic/generic logging helpers (e.g. `LogInfo(logger, some_bool_expr, ...)`). - `std::pair`-style forwarding-reference constructors instantiated with `bool` (e.g. `return {value, overflow_flag};`, `map::insert()`-returned pairs consumed via structured bindings), reported as "Conversion from 'bool' to 'type &'" because the parameter's *un-instantiated* template type is shown. Add the missing exclusion: also strip a top-level reference before the `BoolType` check. Verified locally against a real CodeQL database built from `//score/message_passing` and `//score/mw/com` in eclipse-score/communication: RULE-7-0-1 findings drop from 49 to 5, and the 5 remaining are all genuine bool-to-numeric conversions (`static_cast<uint8_t>(bool)`, etc.) that are unaffected by this change. Extend the regression test with compliant cases for `bool&`, `const bool&`, a forwarding-reference template parameter deduced as `bool`, and `std::pair<T, bool>` construction.
1 parent 4d0376a commit c45b003

4 files changed

Lines changed: 80 additions & 32 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
- `RULE-7-0-1` - `NoConversionFromBool.ql`:
2+
- Fixed false positives where a `bool` value is bound to a reference whose
3+
referenced type is also `bool` (e.g. `bool&`, `const bool&`), including
4+
when this happens via a generic/forwarding-reference parameter (e.g.
5+
`template<class T> void f(T&& t)`, or class template forwarding
6+
constructors such as `std::pair`'s `pair(U1&&, U2&&)`) that happens to be
7+
instantiated with `bool`. Binding a value to a reference of its own type
8+
does not change the type or representation of the value, so this is not
9+
a conversion from `bool` in the sense intended by the rule.

‎cpp/misra/src/rules/RULE-7-0-1/NoConversionFromBool.ql‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ where
2323
conv = e.getConversion() and
2424
conv.getExpr().getType().stripTopLevelSpecifiers() instanceof BoolType and
2525
not conv.getType().stripTopLevelSpecifiers() instanceof BoolType and
26+
// Exclude conversions that only bind a `bool` value to a reference to `bool`
27+
// (e.g. `bool&`, `const bool&`). Binding a value to a reference of its own
28+
// type does not change the type or representation of the value, so this is
29+
// not a "conversion from bool" in the sense intended by the rule. This
30+
// commonly occurs when a `bool` argument is forwarded through a generic
31+
// `bool&`/`const bool&` parameter (e.g. logging helpers, `std::pair`-style
32+
// structured bindings/aggregates).
33+
not conv.getType().stripTopLevelSpecifiers().(ReferenceType).getBaseType().stripTopLevelSpecifiers() instanceof
34+
BoolType and
2635
// Exclude cases that are explicitly allowed
2736
not (
2837
// Exception: equality operators with both bool operands
Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
1-
| test.cpp:25:7:25:8 | b1 | Conversion from 'bool' to 'int'. |
2-
| test.cpp:25:12:25:13 | b2 | Conversion from 'bool' to 'int'. |
3-
| test.cpp:27:7:27:8 | b1 | Conversion from 'bool' to 'int'. |
4-
| test.cpp:27:12:27:13 | b2 | Conversion from 'bool' to 'int'. |
5-
| test.cpp:29:7:29:8 | b1 | Conversion from 'bool' to 'int'. |
6-
| test.cpp:29:12:29:13 | b2 | Conversion from 'bool' to 'int'. |
7-
| test.cpp:31:8:31:9 | b1 | Conversion from 'bool' to 'int'. |
8-
| test.cpp:35:7:35:8 | b1 | Conversion from 'bool' to 'int'. |
9-
| test.cpp:35:12:35:13 | b2 | Conversion from 'bool' to 'int'. |
10-
| test.cpp:37:7:37:8 | b1 | Conversion from 'bool' to 'int'. |
11-
| test.cpp:37:12:37:13 | b2 | Conversion from 'bool' to 'int'. |
12-
| test.cpp:39:7:39:8 | b1 | Conversion from 'bool' to 'int'. |
13-
| test.cpp:39:13:39:14 | b2 | Conversion from 'bool' to 'int'. |
14-
| test.cpp:41:7:41:8 | b1 | Conversion from 'bool' to 'int'. |
15-
| test.cpp:41:13:41:14 | b2 | Conversion from 'bool' to 'int'. |
16-
| test.cpp:45:7:45:8 | b1 | Conversion from 'bool' to 'int'. |
17-
| test.cpp:47:7:47:8 | b1 | Conversion from 'bool' to 'int'. |
18-
| test.cpp:49:7:49:8 | b1 | Conversion from 'bool' to 'int'. |
19-
| test.cpp:53:20:53:21 | b1 | Conversion from 'bool' to 'double'. |
1+
| test.cpp:26:7:26:8 | b1 | Conversion from 'bool' to 'int'. |
2+
| test.cpp:26:12:26:13 | b2 | Conversion from 'bool' to 'int'. |
3+
| test.cpp:28:7:28:8 | b1 | Conversion from 'bool' to 'int'. |
4+
| test.cpp:28:12:28:13 | b2 | Conversion from 'bool' to 'int'. |
5+
| test.cpp:30:7:30:8 | b1 | Conversion from 'bool' to 'int'. |
6+
| test.cpp:30:12:30:13 | b2 | Conversion from 'bool' to 'int'. |
7+
| test.cpp:32:8:32:9 | b1 | Conversion from 'bool' to 'int'. |
8+
| test.cpp:36:7:36:8 | b1 | Conversion from 'bool' to 'int'. |
9+
| test.cpp:36:12:36:13 | b2 | Conversion from 'bool' to 'int'. |
10+
| test.cpp:38:7:38:8 | b1 | Conversion from 'bool' to 'int'. |
11+
| test.cpp:38:12:38:13 | b2 | Conversion from 'bool' to 'int'. |
12+
| test.cpp:40:7:40:8 | b1 | Conversion from 'bool' to 'int'. |
13+
| test.cpp:40:13:40:14 | b2 | Conversion from 'bool' to 'int'. |
14+
| test.cpp:42:7:42:8 | b1 | Conversion from 'bool' to 'int'. |
15+
| test.cpp:42:13:42:14 | b2 | Conversion from 'bool' to 'int'. |
16+
| test.cpp:46:7:46:8 | b1 | Conversion from 'bool' to 'int'. |
17+
| test.cpp:48:7:48:8 | b1 | Conversion from 'bool' to 'int'. |
18+
| test.cpp:50:7:50:8 | b1 | Conversion from 'bool' to 'int'. |
2019
| test.cpp:54:20:54:21 | b1 | Conversion from 'bool' to 'double'. |
21-
| test.cpp:55:28:55:29 | b1 | Conversion from 'bool' to 'int'. |
22-
| test.cpp:58:34:58:35 | b1 | Conversion from 'bool' to 'int8_t'. |
23-
| test.cpp:59:36:59:37 | b1 | Conversion from 'bool' to 'int32_t'. |
24-
| test.cpp:62:6:62:7 | b1 | Conversion from 'bool' to 'int32_t'. |
25-
| test.cpp:63:6:63:7 | b1 | Conversion from 'bool' to 'double'. |
26-
| test.cpp:66:11:66:12 | b1 | Conversion from 'bool' to 'int'. |
27-
| test.cpp:74:9:74:10 | b1 | Conversion from 'bool' to 'int8_t'. |
28-
| test.cpp:75:10:75:11 | b1 | Conversion from 'bool' to 'int32_t'. |
29-
| test.cpp:76:8:76:9 | b1 | Conversion from 'bool' to 'double'. |
30-
| test.cpp:113:33:113:36 | 1 | Conversion from 'bool' to 'A *'. |
31-
| test.cpp:116:8:116:11 | 1 | Conversion from 'bool' to 'int'. |
32-
| test.cpp:117:8:117:12 | 0 | Conversion from 'bool' to 'int'. |
33-
| test.cpp:118:25:118:28 | 1 | Conversion from 'bool' to 'int'. |
20+
| test.cpp:55:20:55:21 | b1 | Conversion from 'bool' to 'double'. |
21+
| test.cpp:56:28:56:29 | b1 | Conversion from 'bool' to 'int'. |
22+
| test.cpp:59:34:59:35 | b1 | Conversion from 'bool' to 'int8_t'. |
23+
| test.cpp:60:36:60:37 | b1 | Conversion from 'bool' to 'int32_t'. |
24+
| test.cpp:63:6:63:7 | b1 | Conversion from 'bool' to 'int32_t'. |
25+
| test.cpp:64:6:64:7 | b1 | Conversion from 'bool' to 'double'. |
26+
| test.cpp:67:11:67:12 | b1 | Conversion from 'bool' to 'int'. |
27+
| test.cpp:75:9:75:10 | b1 | Conversion from 'bool' to 'int8_t'. |
28+
| test.cpp:76:10:76:11 | b1 | Conversion from 'bool' to 'int32_t'. |
29+
| test.cpp:77:8:77:9 | b1 | Conversion from 'bool' to 'double'. |
30+
| test.cpp:114:33:114:36 | 1 | Conversion from 'bool' to 'A *'. |
31+
| test.cpp:117:8:117:11 | 1 | Conversion from 'bool' to 'int'. |
32+
| test.cpp:118:8:118:12 | 0 | Conversion from 'bool' to 'int'. |
33+
| test.cpp:119:25:119:28 | 1 | Conversion from 'bool' to 'int'. |

‎cpp/misra/test/rules/RULE-7-0-1/test.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <cstdint>
2+
#include <utility>
23

34
struct A {
45
explicit A(bool) {}
@@ -122,4 +123,33 @@ void test_bool_conversion_compliant() {
122123

123124
// Bit-field assignment exception - compliant
124125
bf.bit = b1; // COMPLIANT
126+
}
127+
128+
void f3(bool &b) {}
129+
void f4(const bool &b) {}
130+
131+
template <typename T> void f5(T &&t) {}
132+
133+
void test_bool_reference_conversion_compliant() {
134+
bool b1 = true;
135+
136+
// Binding a bool lvalue to a bool reference parameter - compliant, no
137+
// actual type conversion takes place.
138+
f3(b1); // COMPLIANT
139+
140+
// Binding a bool value to a const bool reference parameter - compliant.
141+
f4(b1); // COMPLIANT
142+
f4(true); // COMPLIANT
143+
144+
// Binding a bool value to a forwarding reference parameter deduced as
145+
// bool - compliant.
146+
f5(b1); // COMPLIANT
147+
f5(true); // COMPLIANT
148+
149+
// Binding a bool value through std::pair's generic forwarding-reference
150+
// constructor, where the second template parameter is deduced as bool -
151+
// compliant. This mirrors idiomatic `return {value, overflow_flag};` and
152+
// structured-binding patterns (e.g. std::map::insert()'s return value).
153+
std::pair<int, bool> p1{1, true}; // COMPLIANT
154+
std::pair<int, bool> p2 = {1, b1}; // COMPLIANT
125155
}

0 commit comments

Comments
 (0)