The current SWIG extensions for the string conversion operator is Python specific because it uses the PythonObjects. This means that the code cannot be reused for Lua. I propose to rewrite this extension by using ConstString instead.
Details
- Reviewers
labath - Commits
- rG4d202ac61524: [lldb/SWIG] Refactor extensions to be non Python-specific (3/3)
rG1150259ed146: [lldb/SWIG] Refactor extensions to be non Python-specific (2/2)
rGa59db64a41bd: [lldb/SWIG] Refactor extensions to be non Python-specific
rG51bdd98b8a52: [lldb/SWIG] Refactor extensions to be non Python-specific (3/3)
rGae47a3d81078: [lldb/SWIG] Refactor extensions to be non Python-specific (2/2)
rG0341c11e0850: [lldb/SWIG] Refactor extensions to be non Python-specific
Diff Detail
Event Timeline
lldb/scripts/extensions.swig | ||
---|---|---|
9 ↗ | (On Diff #236722) | I don't think this is the responsibility of the bindings, but for now I've kept it to make this a non-functional change. |
I think this is a great direction to move in. If the extensions are written in C(++) then swig can automatically wrap everything we need.
I am wondering if we can avoid constifying each string we return this way though...
lldb/scripts/extensions.swig | ||
---|---|---|
16–17 ↗ | (On Diff #236763) | Would it be possible to return std::string from here? I don't think this is really a public API (it's not accessible from C++, and swig should convert it into a native string before anything can get its hand on it), so I don't we need to be constrained by the SB API rules. And I believe swig already knows how to handle a std::string... |
4 ↗ | (On Diff #236722) | Yes, but for now I'd put this code into SBTarget.i, since it's clearly SBTarget specific. If we have some generic extension code (produced by factoring this function for instance), then we can put it into some more generic place. |
9 ↗ | (On Diff #236722) | FWIW, I don't find it particularly bad, especially when it gets factored into a function... |
delete?