JonasToth added inline comments.
================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:149
+ const QualType RhsType = Rhs->getType();
+ if (Lhs->isValueDependent() || Rhs->isValueDependent() ||
+ LhsType->isDependentType() || RhsType->isDependentType())
----------------
gchatelet wrote:
> JonasToth wrote:
> > you can shorten this condition with `isDependentType`
> I tried using the following but it crashes when I run clang tidy other our
> codebase:
> ```
> if (LhsType->isDependentType() || RhsType->isDependentType())
> return false;
> ```
>
> Maybe I misunderstood what you suggested?
You understood correct, but I was wrong. Could you please try
`LhsType->isInstantiationDependentType()`, as this should involve all kind of
template surprises.
================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:165
+ if (const auto *CO = llvm::dyn_cast<ConditionalOperator>(Rhs)) {
+ const bool NarrowLhs = diagIfNarrowConstant(
+ Context, CO->getLHS()->getExprLoc(), Lhs, CO->getLHS());
----------------
gchatelet wrote:
> JonasToth wrote:
> > you could remove both temporaries and return directly and then ellide the
> > braces.
> > If you don't like that please remove the `const`
> I have to keep both variables or the first one might shortcircuit the second
> that goes undetected.
> e.g. `unsigned char c = b ? 1 : -1;`
Indeed, no problem.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
