lebedev.ri added a comment. @aaron.ballman thank you so much for the review!
================ Comment at: clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:130 + StringRef TyAsString = + IndexExprType->isSignedIntegerType() ? "ssize_t" : "size_t"; + ---------------- aaron.ballman wrote: > lebedev.ri wrote: > > aaron.ballman wrote: > > > aaron.ballman wrote: > > > > One thing that's awkward about this is that there's no portable > > > > `ssize_t` type -- that's a POSIX type but it doesn't exist on all > > > > platforms (like Windows). We shouldn't print out a typecast that's > > > > going to cause compile errors, but we also shouldn't use the underlying > > > > type for `ssize_t` as that may be incorrect for other target > > > > architectures. > > > I'm still not quite certain what to do about this. Would it make sense to > > > use the underlying type on platforms that don't have `ssize_t`? > > > Relatedly, if we're going to suggest this as a replacement, we should > > > also insert an include for the correct header file. > > I've been thinking about this, and i really can't come up with a better fix > > than using `ptrdiff_t`. > I'm not super excited about using `ptrdiff_t` as there are not likely to be > pointer subtractions in the vicinity of the code being diagnosed, but at the > same time, `ptrdiff_t` is functionally the same as `size_t` except with a > sign bit, which is what `ssize_t` is. I'll hold my nose for now, but if we > get bug reports about this use, we may want to investigate more involved > solutions. Yep. `signed size_t` isn't a valid type, so at best i guess we could avoid emitting such a fixit. In C++, `std::make_signed<size_t>::type` is valid, but it's kinda mouthful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93822/new/ https://reviews.llvm.org/D93822 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
