[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

There are a couple of questions from the previous iteration of the review that aren't answered yet:

- the diagnostic wording doesn't actually say what's wrong with the code or how to fix it; how should users silence the diagnostic if they've found a case where it's a false positive for some reason?
- do you expect this check to catch other hidden provenance gotchas like grabbing values through printf/scanf, fread, etc)? (Not for this patch iteration, just wondering if you expect to extend the check in the future.)



================
Comment at: clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.cpp:21
+  Finder->addMatcher(castExpr(hasCastKind(CK_IntegralToPointer),
+                              unless(hasSourceExpression(integerLiteral())))
+                         .bind("x"),
----------------
Do we also want to exclude the case where the destination is a `volatile` pointer on the assumption that something out of the ordinary is going on. e.g.,
```
intptr_t addr;
if (something) {
  addr = SOME_CONSTANT;
} else {
  addr = SOME_OTHER_CONSTANT;
}
volatile int *register_bank = (volatile int *)addr;
```


================
Comment at: clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp:53
         "performance-no-automatic-move");
+    CheckFactories.registerCheck<NoIntToPtrCheck>("performance-no-inttoptr");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
----------------
Does anyone else have opinions on `inttoptr` vs `int-to-ptr`? I feel like the latter is easier to read than the former, but I'm wondering if I'm just strange.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:15
 
-   `abseil-duration-addition <abseil-duration-addition.html>`_, "Yes"
-   `abseil-duration-comparison <abseil-duration-comparison.html>`_, "Yes"
-   `abseil-duration-conversion-cast <abseil-duration-conversion-cast.html>`_, "Yes"
-   `abseil-duration-division <abseil-duration-division.html>`_, "Yes"
-   `abseil-duration-factory-float <abseil-duration-factory-float.html>`_, "Yes"
-   `abseil-duration-factory-scale <abseil-duration-factory-scale.html>`_, "Yes"
-   `abseil-duration-subtraction <abseil-duration-subtraction.html>`_, "Yes"
-   `abseil-duration-unnecessary-conversion <abseil-duration-unnecessary-conversion.html>`_, "Yes"
-   `abseil-faster-strsplit-delimiter <abseil-faster-strsplit-delimiter.html>`_, "Yes"
+   `abseil-duration-addition <abseil-duration-addition.html>`_,
+   `abseil-duration-comparison <abseil-duration-comparison.html>`_,
----------------
It looks like there's a whole bunch of unrelated changes happening here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055