[clang-tidy] wrong fixup for bugprone-implicit-widening-of-multiplication-result

[Febbe]

$ clang-tidy --version
LLVM (http://llvm.org/):
  LLVM version 14.0.6
  Optimized build.
  Default target: x86_64-pc-windows-msvc
  Host CPU: znver2

clang tidy tries to fix the following:

TEST(test_mem, std_list_in_vmem) {
    monotonic_virtual_memory_resource vmem(1024 * 1024, 1024 * 1024 * 1024);
    {/*<code>*/}

TEST(test_mem, std_list_in_vmem) {
    monotonic_virtual_memory_resource vmem(1024 * 1024, 1024 * 1024 * 1024);
    {/*<code>*/}

// TODO this does not test
TEST(test_mem, std_list_in_vmem) {
    monotonic_virtual_memory_resource vmem(static_cast<size_t>(1024 * )1024, 1024 * 1024 * 1024);
    {

This results in code which is not compilable anymore.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

[bansan85]

I would like to add another comment about the wrong fixup.

#include <cstddef>
#include <cstdint>

  int64_t f(int x, int y) {
    return 1024 * 1024 * 1;
  }

int main() {}

Proposal fix:

#include <cstddef>
#include <cstdint>

  int64_t f(int x, int y) {
    return static_cast<int64_t>(1024 * 1024 * 1024 * )1;
  }

int main() {}

I think the idea is not to fix it with static_cast<int64_t>(1024 * 1024 * 1024 * 1) but static_cast<int64_t>(1024) * 1024 * 1024 * 1

[Febbe]

It would be better to fix it with 1024ULL instead of an extremely verbose static_cast

[v1nh1shungry]

Ｉ submitted a revision trying to fix the wrong fix-hints: https://reviews.llvm.org/D141058. Reviewing and suggestions are welcome!

It would be better to fix it with 1024ULL instead of an extremely verbose static_cast

Since this checker also fixes codes like

long long foobar(int a, int b) {
  return a * b;
  // will give a fix-hint leading to: `return static_cast<long long>(a * b);`
  // if the fix-hint is correct.
}

, I think it's hard for this checker to achieve such a fix. Or the current fix can cover more cases.

[Febbe]

I think it's hard for this checker to achieve such a fix.

It's relatively easy to determine, if an expression is derived from an literal. In the case of an literal just propose another fix.

[crisluengo]

I would suggest that 1024 * 1024 does not need any changes. Yes, the result will be widened, but it's a constant value, and we know there is not going to be any overflow in the computation. This code should not fire the warning at all, it should only fire if a variable is involved, or if the computation actually produces the wrong result because of overflow.