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