lebedev.ri added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + ---------------- martong wrote: > martong wrote: > > What is the difference between "default" and "fundamental" alignment? Are > > they the same? Can they differ in any architecture? > > > > https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM57-CPP.+Avoid+using+default+operator+new+for+over-aligned+types > > Here there is no wording of "default alignment" only "fundamental > > alignment" is mentioned. Based on this I'd call this as > > `FundamentalAligment`. > > What is the difference between "default" and "fundamental" alignment? Are > > they the same? > > `fundamental alignment` of any type is the alignment of std::max_align_t. > I.e. `alignof(std::max_align_t)`. > See C++17 6.11.2. > > On the other hand, default alignment is the value in > `__STDCPP_DEFAULT_NEW_ALIGNMENT__` which may be predefined with > `fnew-alignment` > See https://www.bfilipek.com/2019/08/newnew-align.html > > These values can differ: https://wandbox.org/permlink/yIwjiNMw9KyXEQan > > Thus, I think we should use the fundamental alignment here, not the default > alignment. > So, `getNewAlign()` does not seem right to me. > @aaron.ballman What do you think? > Thus, I think we should use the fundamental alignment here, not the default > alignment. I have the exact opposite view. If as per `getNewAlign()` the alignment would be okay, why should we not trust it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D67545 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
