Page MenuHomePhabricator

[lldb/Lua] add initial Lua typemaps
ClosedPublic

Authored by tammela on Jan 18 2021, 4:38 PM.

Details

Summary

This patch adds the integer handling typemaps and the typemap for
string returning functions.

The integer handling typemaps overrides SWIG's own typemaps to distinct
the handling of integers from floating point.

The typemap for string returning functions is a port of Python's
typemap.

Diff Detail

Event Timeline

tammela requested review of this revision.Jan 18 2021, 4:38 PM
tammela created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 18 2021, 4:38 PM
JDevlieghere requested changes to this revision.Jan 18 2021, 4:40 PM

We guarantee that the SB API is ABI stable so you cannot change the signature of existing functions. Would an overload do the trick?

This revision now requires changes to proceed.Jan 18 2021, 4:40 PM
tammela updated this revision to Diff 317439.Jan 18 2021, 4:43 PM

Removing spurious line

We guarantee that the SB API is ABI stable so you cannot change the signature of existing functions. Would an overload do the trick?

Hmmm, that's tricky. I will see if I can come up with something.

We guarantee that the SB API is ABI stable so you cannot change the signature of existing functions. Would an overload do the trick?

If overloaded with const char *GetStringValue(const char *fail_value = nullptr), SWIG emits a warning when building the Python wrapper saying that the method is shadowed and doesn't emit it.
const char *GetStringValue() works, however it doesn't follow the API contract.
Would adding a new method GetConstStringValue() also be acceptable?

I'm out of ideas to solve this. This method is very Python oriented, as the SWIG wrapper understands the {in,out} documentation. This doesn't happen for the Lua wrapper.

I replied before I actually tried to understand what your'e trying to achieve, and it's still not entirely clear to me. I think what you need is something similar like python-typemaps.swig which helps swig understand char** types and such.

For context, originally SWIG wasn't great at handling std::strings (which has since been fixed) and the designers of the SB API were worried about the ABI stability of the std::string class. None of these are concerns anymore today, but the ABI being stable means we're stuck with it for now. If we ever do an SB API v2, this is definitely one of the things we'd fix, but currently there are no plans for that.

I replied before I actually tried to understand what your'e trying to achieve, and it's still not entirely clear to me. I think what you need is something similar like python-typemaps.swig which helps swig understand char** types and such.

For context, originally SWIG wasn't great at handling std::strings (which has since been fixed) and the designers of the SB API were worried about the ABI stability of the std::string class. None of these are concerns anymore today, but the ABI being stable means we're stuck with it for now. If we ever do an SB API v2, this is definitely one of the things we'd fix, but currently there are no plans for that.

That's exactly what I need, thanks. I will adjust accordingly.

tammela updated this revision to Diff 318298.Jan 21 2021, 1:16 PM

Addressing comments

tammela retitled this revision from [lldb] change SBStructuredData GetStringValue signature to [lldb/Lua] add initial Lua typemaps.Jan 21 2021, 1:18 PM
tammela edited the summary of this revision. (Show Details)
JDevlieghere accepted this revision.Jan 22 2021, 11:01 AM

LGTM

lldb/bindings/lua/lua-typemaps.swig
7

nit: Lua 5.3

This revision is now accepted and ready to land.Jan 22 2021, 11:01 AM
This revision was automatically updated to reflect the committed changes.