mizvekov marked 7 inline comments as done. mizvekov added a subscriber: sammccall. mizvekov added a comment. Herald added a subscriber: steakhal. Herald added a reviewer: NoQ.
In D112374#3535949 <https://reviews.llvm.org/D112374#3535949>, @rsmith wrote: > I've not looked at the test changes in any detail; please let me know if > there's anything in there that deserves special attention. The new (since last rebase) change in `clang-tools-extra/test/clang-tidy/checkers/bugprone/shared-ptr-array-mismatch.cpp` might be worth a look. I think this is a fix, and the pre-existing code missed it by pure accident. I am not sure there are any scenarios where we might want to avoid suggesting this fix in dependent contexts, if there were we should throw a FIXME in there. There are a couple of minor significant changes since the last diff, might be worth an extra look, but mainly some minor conflicts with the introduction of UsingType by @sammccall . ================ Comment at: llvm/include/llvm/Support/SizeOf.h:20-23 +/// A sizeof operator stand-in which supports `sizeof(void) == 0`. +/// +template <class T> struct SizeOf { static constexpr size_t value = sizeof(T); }; +template <> struct SizeOf<void> { static constexpr size_t value = 0; }; ---------------- rsmith wrote: > I think it's too surprising to call this simply `SizeOf`, especially given > that under > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0146r1.html we > might eventually see C++ defining `sizeof(void)` as some non-zero value. The author merely suggests the two issues are independent :) If this paper ever gets followed up on, leaving `sizeof(void) != 0` would be a riot! ================ Comment at: llvm/include/llvm/Support/SizeOf.h:25-32 +/// An alignof operator stand-in which supports `alignof(void) == 1`. +/// +template <class T> struct AlignOf { + static constexpr size_t value = alignof(T); +}; +template <> struct AlignOf<void> { static constexpr size_t value = 1; }; + ---------------- rsmith wrote: > There's already a (different!) `llvm::AlignOf` in `llvm/Support/AlignOf.h`. Well that is the one which is surprising to simply call `AlignOf` =) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
