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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
