[PATCH] D154893: [Clang] Fix some triviality computations

Shafik Yaghmour via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 24 14:30:18 PDT 2023


shafik added inline comments.


================
Comment at: clang/include/clang/AST/DeclCXX.h:1269
   /// Determine whether this class has a non-trivial copy constructor
   /// (C++ [class.copy]p6, C++11 [class.copy]p12)
   bool hasNonTrivialCopyConstructor() const {
----------------
philnik wrote:
> royjacobson wrote:
> > shafik wrote:
> > > These references looks like they need to be updated. Same below and it looks like `hasNonTrivialCopyConstructorForCall` is missing references all together.
> > TBH I don't think those functions actually need references to the standard? Whether the actual member functions are trivial or not is already calculated before. Do you think I can just remove it? :)
> I think it makes sense to call out that these functions represent something from the standard. There are other functions with similar names, which don't have an equivalent in the C++ standard, like `hasTrivialDestructorForCall`.
The point of having the reference is so folks can understand the logic behind why someone choose to implement the functionality as they did.

I also prefer quoting the critical text since the paragraphs, sometimes the stable name and wording can change and at least having text can allow us to track which standard it came from and hopefully which change changed the text and understand if it effects our compliance or not.

It can be a lot of work to figure this out from scratch if you don't know where to look.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154893



More information about the cfe-commits mailing list