[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

Sat Jan 7 05:14:41 PST 2023

Febbe added a comment.

In D141058#4028768 <https://reviews.llvm.org/D141058#4028768>, @v1nh1shungry wrote:

> Sorry, I don't know how to add tests for such fixes. Could anyone please give me some hints? Thanks!

It seems like the tests for this check do not contain checks for the fixups itself.

Normally, they are tested with `CHECK-FIXES: <Fixed line above, whitespaces truncated at the beginning and the end>` for example:

  return F? Mov: Movable{}; 
  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use] 
  // CHECK-FIXES: return F? std::move(Mov): Movable{};

In the case of this test file, you have to append the suffixes to the `CHECK-FIXES` to `CHECK-FIXES-<SUFFIX>` where `<SUFFIX>` is one of `C`, `CXX`, `ALL`

  return F? Mov: Movable{}; 
  // CHECK-MESSAGES-CXX: [[@LINE-1]]:13: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use] 
  // CHECK-FIXES-CXX: return F? std::move(Mov): Movable{};

================
Comment at: clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:215
                   (Twine("static_cast<") + TyAsString + ">(").str())
-           << FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
     else
----------------
Actually, I find it pretty weird/suspicious, that `<AnyBinaryExr*> -> getEndLoc()` does not return the end of its `Expr`. The code looked correct already.
Can you elaborate, if this is a bug in the `getEndLoc()` of those `Expr`s? It might be better to fix it there directly.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141058/new/

https://reviews.llvm.org/D141058