lawrence_danna added a comment. In D68546#1699397 <https://reviews.llvm.org/D68546#1699397>, @labath wrote:
> Yeah, the %extends are somewhat ugly, but I also find the FileSP overloads in > the "public" sections of the SB classes unnerving too, because they are not > really public -- no user can actually call them directly in a meaningful way. > They are really only meant for the python interface, which sort does sit on > the other side of the SB api, but also kind of doesn't. That's the weird part > about lldb python support, where python can be embedded *into* lldb, but it > also can be sitting *above* it (or both at the same time). I am not really > sure what to do about that. I don't suppose there's any way to make the > FileSP overloads private? I don't think so, because the SWIG wrappers need to see them. They are de-facto private though, because they require an argument of type `std::shared_ptr<lldb_private::File>`. All external API clients can see is a forward declaration that lldb_private::File is a class. They can make a NULL pointer to one, but that's it. > Or maybe they actually should be public, and the actual problem is that there > is no way for a c++ user to make use of this functionality, even though it > should be able to? Right now, the c++ api is sort of disadvantaged in that > one cannot create an SBFile that would redirect to some custom fancy c++ > output object. I am not saying you should implement that, but if we assume > that such functionality were available do you think it would be possible to > implement the python stuff on top of that? In that sense, maybe the FileSP > overloads are actually fine, and we can treat them as a placeholder for this > hypothetical future functionality? > > I am sorry if this doesn't make much sense -- that's because I myself am not > clear on this matter. But I hope that I at least managed to give you an idea > of the things going through my head right now. If you have any thoughts on > any of this, I'd like to hear them. No, it does make sense. I too thought it was kind of odd that there's a python API which is inaccessible from C++. I can't really think of any ultimate use case where a C++ binding for File I/O callbacks would be needed, but if it were, I can think of three ways to do it. The first and simplest way to support it is to just use `funopen()` or whatever the equivalent of that is on your platform to make a `FILE*`. I put the `FILE*` constructor in for C++ even though it will never have a SWIG wrapper so this would be possible. The disadvantage is that `funopen()` isn't portable, so clients that go this route will need platform specific code to do it. The intermediate method would be to make our own funopen that would act as a factory function for `FileSP`. File would remain private, but you could make one with callbacks if you wanted to. Disadvantage here is that C++ clients would be calling though a chain of two function pointers to actually get to the Read and Write methods. This is still better from a performance perspective than calling into a Python method, but if we wanted to avoid that, we could go one step further. The most complicated way we could handle it would be to expose an ABI-stable abstract base class of `File` in the API headers. This should be at the base of the File class hierarchy, where `IOObject` is now. It would only have virtual methods, and it would also have a bunch of unused dummy virtual so we can add new virtual methods later if we need too, without changing the ABI. C++ clients could then just subclass their own `File` types and use `FileSP` directly. I think to do this properly we'd need to do some more refactoring, because I'm not sure what should happen to `IOObject` and `Socket` under this scenario, or what kinds of error codes the virtual methods should be returning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68546/new/ https://reviews.llvm.org/D68546 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
