[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

Thu Feb 2 19:09:16 PST 2023

kastiglione added inline comments.

================
Comment at: lldb/include/lldb/API/SBAddress.h:14-16
+#ifdef SWIG
+%include "interface/SBAddressDocstrings.i"
+#endif
----------------
bulbazord wrote:
> clayborg wrote:
> > bulbazord wrote:
> > > clayborg wrote:
> > > > Do we want two different .i files? Right now we have both"interface/SB*Docstrings.i" and "interface/SB*Extensions.i". Can those be combined into just one "interface/SBAddress.i" and only included if we have anything in the .i file that isn't just a copy of the API itself? Or does the doc string stuff have to come first?
> > > What I discovered when working this patch is that the docstring information needs to come before the declaration of what it wants to document. So `SB*Docstrings.i` should come first. The `SB*Extensions.i` are there to extend an existing class, so they should come after the class itself. I'm not sure there's a good way to do this.
> > > 
> > > Alternatively, we could changes the `interfaces.swig` file to include `./interface/SBAddressDocstrings.i`, then `lldb/API/SBAddress.h`, then `./interface/SBAddressExtensions.i`. That way we don't need to have these 2 `#ifdef` blocks in the API headers themselves. 
> > Anything we can do to keep the headers clean would be great. I know that was the original reason we started using them.
> I'll try to minimize the noise in the API headers when I do the complete thing.
> Alternatively, we could changes the `interfaces.swig` file to include `./interface/SBAddressDocstrings.i`, then `lldb/API/SBAddress.h`, then `./interface/SBAddressExtensions.i`. That way we don't need to have these 2 `#ifdef` blocks in the API headers themselves. 

I was going to make this suggestion. I think it's better to do that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926