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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits