[PATCH] D71707: clang-tidy: new bugprone-pointer-cast-widening

[Jan Kratochvil via Phabricator via cfe-commits]

jankratochvil marked 8 inline comments as done.
jankratochvil added a comment.

In D71707#1796671 <https://reviews.llvm.org/D71707#1796671>, @aaron.ballman wrote:

> I agree that restricting casts to `intptr_t` is too restrictive.


Permitted `intptr_t`. But then implemented also checking a cast of `intptr_t` to wider integral types.

> Have you considered language extensions like `__ptr32`, `__ptr64`, `__sptr`, and `__uptr` (https://docs.microsoft.com/en-us/cpp/cpp/ptr32-ptr64?view=vs-2019) which we also support?

I think only `__uptr` support would matter to prevent some false positives. I am not sure it is used anywhere. Should I really implement it?



================
Comment at: clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.cpp:21-22
+void PointerCastWideningCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus && !getLangOpts().C99)
+    return;
+
----------------
aaron.ballman wrote:
> Why limiting this to C++ and >= C99? You can get pointer widening casts through extensions in C89, for instance.
Removed the check completely.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.cpp:36
+  for (QualType DestTypeCheck = DestType;;) {
+    if (DestTypeCheck.getAsString() == "uintptr_t")
+      return;
----------------
aaron.ballman wrote:
> How well does this work with something like `std::uinptr_t` from `<cstdint>`?
It works fine because its
```
using::uintptr_t;
```
is recogized as a `typedef` in the loop above from namespace-less `uintptr_t'. Sure thanks for notifying me to check that.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-pointer-cast-widening.cpp:17
+  t2 t = (t2)p;
+}
----------------
aaron.ballman wrote:
> I'd also like to see tests for implicit casts as well as pointer width and sign extension language extensions.
Done implicit cases.  But I do not understand what you mean by the rest - could you provide an example? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71707