courbet added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:104
+ castExpr(hasCastKind(CK_LValueToRValue),
+ has(memberExpr(hasDeclaration(fieldDecl(isBitField()))))))),
+ hasParent(
----------------
There needs to be some kind of check of the size of the bit field vs the size
of the integer, because `struct SmallBitfield { unsigned int id : 31; };`
should warn.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:105
+ has(memberExpr(hasDeclaration(fieldDecl(isBitField()))))))),
+ hasParent(
+ binaryOperator(anyOf(hasOperatorName("<<"),
hasOperatorName(">>")))));
----------------
What is specific about shifting operators ? What about `x+1` for example ? You
might have opened a pandora box here: you'll need to tech the check about the
semantics of all binary operations.
```
BinaryOperator <line:7:3, col:10> 'int' '+'
|-ImplicitCastExpr <col:3, col:5> 'int' <IntegralCast>
| `-ImplicitCastExpr <col:3, col:5> 'unsigned int' <LValueToRValue>
| `-MemberExpr <col:3, col:5> 'unsigned int' lvalue bitfield .id
0x55e144e7eaa0
| `-DeclRefExpr <col:3> 'SmallBitfield' lvalue Var 0x55e144e7eb18 'x'
'SmallBitfield'
`-IntegerLiteral <col:10> 'int' 1
```
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:106
+ hasParent(
+ binaryOperator(anyOf(hasOperatorName("<<"),
hasOperatorName(">>")))));
+
----------------
There also needs to be some check of the constant value of the RHS, because
`x.id << 30` can actually overflow, so we want to warn in that case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114105/new/
https://reviews.llvm.org/D114105
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
