[PATCH] D53488: [clang-tidy] Improving narrowing conversions
Guillaume Chatelet via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 26 01:06:49 PST 2018
gchatelet added a comment.
Thank you for bearing with me @aaron.ballman.
================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:259-263
+void NarrowingConversionsCheck::handleIntegralToBoolean(
+ const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
+ const Expr &Rhs) {
+ // Conversion from Integral to Bool value is well defined.
+}
----------------
aaron.ballman wrote:
> Why is this function needed at all?
The two empty functions are placeholders so the cases handled by `ImplicitCast` and `BinaryOp` are the same.
I'll add some documentation.
================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:311-315
+void NarrowingConversionsCheck::handleBooleanToSignedIntegral(
+ const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
+ const Expr &Rhs) {
+ // Conversion from Bool to SignedIntegral value is well defined.
+}
----------------
aaron.ballman wrote:
> Why is this function needed at all?
Ditto.
================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:372
+ if (const auto *CO = llvm::dyn_cast<ConditionalOperator>(&Rhs)) {
+ // We create variables so we make sure both sides of the
+ // ConditionalOperator are evaluated, the || operator would short ciruit
----------------
aaron.ballman wrote:
> What variables are created here?
Leftover. I rewrote the comment.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53488/new/
https://reviews.llvm.org/D53488
More information about the cfe-commits
mailing list