alexfh added a comment.
In https://reviews.llvm.org/D38455#1061195, @courbet wrote:
> ping
Sorry for the long review due to the holidays.
Generally, I would also like Aaron to take a look when he's back, since he had
some concerns. While we're waiting, one minor comment from me.
================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:41
+ binaryOperator(
+ anyOf(hasOperatorName("+="), hasOperatorName("-=")),
+ hasLHS(hasType(isInteger())),
----------------
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.
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
