[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.


================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:41
+      binaryOperator(
+          anyOf(hasOperatorName("+="), hasOperatorName("-=")),
+          hasLHS(hasType(isInteger())),
----------------
alexfh wrote:
> JonasToth wrote:
> > courbet wrote:
> > > JonasToth wrote:
> > > > I would really like to add all other calc-and-assign operations. At least "*=" and "/=".
> > > Makes sense, I've added `*=` and `/=`. All others (`%=`, `&=`, `<<=` and variations thereof)  trigger a compilation error if the RHS is a float (rightfully).
> > > 
> > > 
> > For floats that is true.
> > 
> > If we care about the `ìnt->char` casts later, they should be added.
> Does it make sense to match all assignment operators including just `=`? If so, the matcher will become a bit simpler: `binaryOperator(isAssignmentOperator(), ...)`. If there's a reason not to do this, maybe adding a comment would be useful
> 
> Apart from that, I wonder whether the matcher above would also cover the case of assignment operators? I would expect the AST for assignment expressions with different types to have ImplicitCast nodes anyway.
> If we care about the ìnt->char casts later, they should be added.

Will do. I'd like to start by implementing only floats first though, because they are the ones where I have data.

> Apart from that, I wonder whether the matcher above would also cover the case of assignment operators?

It does cover `=` (which generates an implicit cast), but not `+=`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38455