Skip to content

Commit 672571f

Browse files
Merge pull request #1178 from castler/fix_7_0_4_overloaded_operators_fp
Fix RULE-7-0-4 false positives for non-built-in operators
2 parents c281dbd + a0f0606 commit 672571f

4 files changed

Lines changed: 86 additions & 6 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
- `RULE-7-0-4` - `InappropriateBitwiseOrShiftOperands.ql`:
2+
- Fixes #1177 - the rule no longer reports operands whose type is not a MISRA numeric type. The
3+
operand checks used `not isUnsignedType(operandType)`, which is vacuously true for every type
4+
that has no MISRA numeric type at all, such as class types and the unresolved dependent types
5+
of uninstantiated template bodies. As a result the rule reported operations that do not use
6+
the built-in operators, most notably the stream insertion and extraction operators. The checks
7+
now use `isSignedType(operandType)` instead.
8+
- Operands of character type, of a non-standard integral type, and of an unscoped enumeration
9+
type without a fixed underlying type are consequently no longer reported, because none of them
10+
has a MISRA numeric type. Unscoped enumerations without a fixed underlying type are covered by
11+
`RULE-10-2-3`.

cpp/misra/src/rules/RULE-7-0-4/InappropriateBitwiseOrShiftOperands.ql

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ where
6363
|
6464
x = op.getLeftOperand() and
6565
operandType = op.getLeftOperand().getExplicitlyConverted().getType() and
66-
not MisraCpp23BuiltInTypes::isUnsignedType(operandType) and
66+
MisraCpp23BuiltInTypes::isSignedType(operandType) and
6767
message =
6868
"Bitwise operator '" + op.getOperator() +
6969
"' requires unsigned numeric operands, but the left operand has type '" + operandType +
7070
"'."
7171
or
7272
x = op.getRightOperand() and
7373
operandType = op.getRightOperand().getExplicitlyConverted().getType() and
74-
not MisraCpp23BuiltInTypes::isUnsignedType(operandType) and
74+
MisraCpp23BuiltInTypes::isSignedType(operandType) and
7575
message =
7676
"Bitwise operator '" + op.getOperator() +
7777
"' requires unsigned numeric operands, but the right operand has type '" + operandType +
@@ -82,7 +82,7 @@ where
8282
exists(ComplementExpr comp, Type opType |
8383
x = comp.getOperand() and
8484
opType = comp.getOperand().getExplicitlyConverted().getType() and
85-
not MisraCpp23BuiltInTypes::isUnsignedType(opType) and
85+
MisraCpp23BuiltInTypes::isSignedType(opType) and
8686
message =
8787
"Bit complement operator '~' requires unsigned operand, but has type '" + opType + "'."
8888
)
@@ -91,7 +91,7 @@ where
9191
exists(BinaryShiftOpOrAssignOp shift, Type leftType |
9292
x = shift.getLeftOperand() and
9393
leftType = shift.getLeftOperand().getExplicitlyConverted().getType() and
94-
not MisraCpp23BuiltInTypes::isUnsignedType(leftType) and
94+
MisraCpp23BuiltInTypes::isSignedType(leftType) and
9595
not isSignedConstantLeftShiftException(shift) and
9696
message =
9797
"Shift operator '" + shift.getOperator() +
@@ -112,7 +112,7 @@ where
112112
"Shift operator '" + shift.getOperator() + "' shifts by " + right.getValue().toInt() +
113113
" which is not within the valid range 0.." + ((leftType.getSize() * 8) - 1) + "."
114114
else (
115-
not MisraCpp23BuiltInTypes::isUnsignedType(rightType) and
115+
MisraCpp23BuiltInTypes::isSignedType(rightType) and
116116
message =
117117
"Shift operator '" + shift.getOperator() +
118118
"' requires unsigned right operand, but has type '" + rightType + "'."

cpp/misra/test/rules/RULE-7-0-4/InappropriateBitwiseOrShiftOperands.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,6 @@
4747
| test.cpp:156:3:156:12 | 1073741824 | Shift operator '<<' requires unsigned left operand, but has type 'int'. |
4848
| test.cpp:162:3:162:5 | s32 | Shift operator '<<' requires unsigned left operand, but has type 'int32_t'. |
4949
| test.cpp:170:3:170:5 | s32 | Shift operator '>>' requires unsigned left operand, but has type 'int32_t'. |
50+
| test.cpp:201:3:201:7 | value | Shift operator '<<' requires unsigned left operand, but has type 'signed int'. |
51+
| test.cpp:226:3:226:4 | e3 | Bitwise operator '&' requires unsigned numeric operands, but the left operand has type 'UnscopedEnumSignedUnderlyingType'. |
52+
| test.cpp:226:7:226:8 | e3 | Bitwise operator '&' requires unsigned numeric operands, but the right operand has type 'UnscopedEnumSignedUnderlyingType'. |

cpp/misra/test/rules/RULE-7-0-4/test.cpp

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,70 @@ void test_right_shift_signed_operands() {
168168

169169
u32 >> 1U; // COMPLIANT
170170
s32 >> 1U; // NON_COMPLIANT
171-
}
171+
}
172+
173+
class TestStream {
174+
public:
175+
TestStream &operator<<(std::int32_t value);
176+
TestStream &operator>>(std::int32_t &value);
177+
};
178+
179+
void test_overloaded_shift_operators() {
180+
TestStream stream;
181+
std::int32_t s32 = 1;
182+
183+
// User provided operators, not the built-in shift operators
184+
stream << 1; // COMPLIANT
185+
stream << s32; // COMPLIANT
186+
stream >> s32; // COMPLIANT
187+
}
188+
189+
template <typename T>
190+
void test_overloaded_shift_operators_in_template(TestStream &stream,
191+
const T &value) {
192+
// The left operand of the second `<<` is the `TestStream &` returned by the
193+
// first one, and the right operand is dependent, so the operation is
194+
// unresolved in the uninstantiated template body
195+
stream << 1 << value; // COMPLIANT
196+
}
197+
198+
template <typename T> void test_dependent_shift_operands(T value) {
199+
// Dependent operands are unresolved in the uninstantiated template body, but
200+
// reported through the instantiation below
201+
value << 2; // NON_COMPLIANT
202+
}
203+
204+
void test_template_instantiations() {
205+
TestStream stream;
206+
test_overloaded_shift_operators_in_template(stream, 1);
207+
test_dependent_shift_operands<std::int32_t>(1);
208+
}
209+
enum UnscopedEnumNoFixedUnderlyingType { EnumeratorA = 1, EnumeratorB = 2 };
210+
211+
enum UnscopedEnumUnsignedUnderlyingType : unsigned int { EnumeratorC = 1 };
212+
213+
enum UnscopedEnumSignedUnderlyingType : int { EnumeratorD = 1 };
214+
215+
void test_enum_operands() {
216+
UnscopedEnumNoFixedUnderlyingType e1 = EnumeratorA;
217+
UnscopedEnumUnsignedUnderlyingType e2 = EnumeratorC;
218+
UnscopedEnumSignedUnderlyingType e3 = EnumeratorD;
219+
220+
// Without a fixed underlying type the enum has no MISRA numeric type, so the
221+
// operands are not analysed by this rule
222+
e1 &e1; // COMPLIANT
223+
224+
e2 &e2; // COMPLIANT
225+
226+
e3 &e3; // NON_COMPLIANT
227+
}
228+
229+
void test_character_type_operands() {
230+
char32_t c32 = 1;
231+
232+
// `char32_t` is of character type, not of numeric type, and is always
233+
// unsigned
234+
c32 &c32; // COMPLIANT
235+
c32 << 1U; // COMPLIANT
236+
~c32; // COMPLIANT
237+
}

0 commit comments

Comments
 (0)