[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

Roy Jacobson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 13 10:50:37 PDT 2022


royjacobson added inline comments.


================
Comment at: clang/bindings/python/clang/cindex.py:1530
+
+    def record_needs_implicit_default_constructor(self):
+        """Returns True if the cursor refers to a C++ record declaration
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > I don't think we should expose any of the "needs" functions like this -- those are internal implementation details of the class and I don't think we want to calcify that into something we have to support forever. As we add members to a class, we recalculate whether the added member causes us to delete defaulted special members (among other things), and the "needs" functions are basically used when the class is completed to handle lazily created special members. I'm pretty sure that lazy creation is not mandated by the standard, which is why I think the "needs" functions are more of an implementation detail.
> CC @erichkeane and @royjacobson as folks who have been in this same area of the compiler to see if they agree or disagree with my assessment there.
I think so. The 'needs_*' functions query `DeclaredSpecialMembers` and I'm pretty sure it's modified when we add the implicit definitions in the class completion code. So this looks a bit suspicious. Is this API //meant// to be used with incomplete classes?
For complete classes I think looking up the default/move/copy constructor and calling `isImplicit()` is the way to do it.

About the 'is deleted' API - can't the same be done for those functions as well so we have a smaller API? 

If this //is// meant to be used with incomplete classes for efficiency that would be another thing, I guess.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135557



More information about the cfe-commits mailing list