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

Reply via email to