[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

Daniel Marjamäki via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 22 06:38:11 PDT 2017


danielmarjamaki added a comment.

In https://reviews.llvm.org/D31097#707385, @baloghadamsoftware wrote:

> Hi!
>
> There is an option to disable the checking of widening casts. It is enabled by default. You can disable it any time. Or, if you find too much false positives, we can discuss about setting this option to disabled as default.
>
> I am convinced that checking implicit widening casts are also necessary. We should probably change the error message in the implicit case from "misplaced" to "missing", and maybe also rename the checker itself. Separating it to two different checkers, which are almost copy of each other is huge code duplication.


It would help to disable it by default and changing the message. But also I believe it's philosophically different to the original checker.

I would say that your logic is more philosophically similar to Wconversion. Could it be added there instead?

Did you try this check on real code? Do you think there is a problem that should be fixed here?

  void foo(unsigned int N) {
    long L;
    if (N<10)
      L = N << 8;
    ...

I am assuming that such code is not uncommon. If you don't think there is a problem in that code then I would personally suggest that you update the ConversionChecker in the analyzer instead.

I do believe that a warning about that code is a false positive.

The idea with the misc-misplaced-widening-cast is that if the developer writes such code:

  void foo(unsigned int N) {
    long L;
    if (N<10)
      L = (long)(N << 8);
    ...

Then there is a message "either cast is misplaced or there is truncation". In this case the cast is misplaced and it can be removed.


Repository:
  rL LLVM

https://reviews.llvm.org/D31097





More information about the cfe-commits mailing list