anderslanglands 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
----------------
dblaikie wrote:
> aaron.ballman wrote:
> > royjacobson wrote:
> > > 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.
> > CCing folks who may have more historical memory of the C APIs and whether 
> > they're expected to operate on a completed AST or are expected to work on 
> > an AST as it is under construction. My unverified belief is that these APIs 
> > are expected to work on a completed AST.
> > 
> > @echristo @dblaikie @rjmccall @rsmith
> > 
> > I'm also not certain of what the best path forward is here. I'm not 
> > comfortable exposing the needs* functions because they really are 
> > implementation details and I don't want to promise we'll support that API 
> > forever. But at the same time, the use case is reasonably compelling on the 
> > assumption you need to inspect the AST nodes as they're still under 
> > construction instead of inspecting them once the AST is completed. If the 
> > AST is fully constructed, then we should have already added the AST nodes 
> > for any special member functions that needed to be generated implicitly, so 
> > as Roy mentioned, you should be able to find the special member function 
> > you're after and check `isImplicit()` on it.
> Not sure I'm quite following - it doesn't look (admittedly, sorry, at a 
> somewhat superficial look at the discussion here) like this is necessarily 
> about incomplete AST - could parse the header and stop. That's a complete 
> AST, yeah? And then it might be OK/reasonable to ask "could this type be 
> default constructed" (even if the implicit ctor has been implicitly 
> instantiated/there was no use in the source code that's been parsed)
> 
> Is that correct?
I am just parsing the headers of a library using `clang_parseTranslationUnit()` 
then using `clang_visitChildren()` to inspect the AST. Doing this I do NOT see 
any implicitly generated methods, hence why I need these functions. It sounds 
like you expect those methods to be in the AST already? Which suggests I'm 
doing something wrong in my parsing (missing another function call/option or 
something)?


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