[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Since I seem to be in the minority about thinking that this check does not pull its weight, I reviewed the code, and will LGTM and push once the few small issues are fixed.



================
Comment at: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp:36
+
+  return FixItHint::CreateReplacement(
+      Outer->getSourceRange(),
----------------
We usually try to make the replacement as small as possible. In the case of this checker, the replacement should move the closing parenthesis or the closing bracket after the RHS of the binary expression.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp:73
+  const auto CXXConstructorWithSingleIntArg =
+    cxxConstructExpr(argumentCountIs(1), hasArgument(0, IntExpr));
+
----------------
It does not have to be a single argument, the same typo can happen with the last argument of the constructor:

```
A *ptr = new A(a, b, c) + 1; // as written
A *ptr = new A(a, b, c + 1); // should have been
```


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.c:12
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = \(char \*\)malloc\(n}} + 10{{\);$}}
+
----------------
I don't think the check lines in this file need regexes. You can match literal text and avoid having to escape special characters:

CHECK-FIXES: char *p = ...;


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

https://reviews.llvm.org/D71001