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 ---------------- anderslanglands wrote: > royjacobson wrote: > > anderslanglands wrote: > > > royjacobson wrote: > > > > 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. > > > > > > > So the intended use case here is I'm using libclang to parse an existing > > > C++ libray's headers and generate a C interface to it. To do that I need > > > to know if I need to generate default constructors etc, which the needs* > > > methods do for me (I believe). The alternative is I have to check > > > manually whether all the constructors/assignment operators exist, then > > > implement the implicit declaration rules myself correctly for each > > > version of the standard, which I'd rather avoid. > > > > > > Would putting a note in the doc comment about the behaviour differing > > > when the class is being constructed as originally suggested work for > > > everyone? > > Why is the `__is_default_constructible` builtin type trait not enough? Do > > you have different behavior for user provided and implicit default > > constructors? > > > Can I evaluate that from libclang somewhow? I can't modify the C++ libraries > I'm wrapping. > > Basically, given: > ``` > struct Foo { /* ... */ }; > ``` > > I want to generate: > > ``` > typedef struct Foo_t; > > Foo_t* Foo_ctor(); > Foo_t* Foo_copy_ctor(Foo_t*); > /* etc... */ > Foo_dtor(Foo_t*); > ``` > > In order to know which ones to generate for an arbitrary struct that may or > may not have any combination of ctor/assignments defined, I need to know > which ones exist and follow the implicit generation rules for the ones that > don't. I can do this myself with a whole bunch of version-dependent logic, > but I'd rather just rely on libclang since it already knows all this much > better than I do. I looked a bit, and it seems they aren't, and that generally libclang doesn't really know about Sema, so exporting the type traits is not that easy :/ I'm not sure what's the best way forward here, but I don't like the idea of exporting those half baked internal API calls when there are actual standardized and implemented type traits that perform the same goal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135557/new/ https://reviews.llvm.org/D135557 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits