[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 318298.
tammela added a comment.

Addressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/lua/lua-typemaps.swig

Index: lldb/bindings/lua/lua-typemaps.swig
===
--- lldb/bindings/lua/lua-typemaps.swig
+++ lldb/bindings/lua/lua-typemaps.swig
@@ -1 +1,97 @@
 %include 
+
+// FIXME: We need to port more typemaps from Python
+
+//===--===//
+
+// In 5.3 and beyond the VM supports integers, so we need to remap
+// SWIG's internal handling to integers.
+
+
+%define LLDB_NUMBER_TYPEMAP(TYPE)
+
+// Primitive integer mapping
+%typemap(in,checkfn="lua_isinteger") TYPE
+%{ $1 = (TYPE)lua_tointeger(L, $input); %}
+%typemap(in,checkfn="lua_isinteger") const TYPE&($basetype temp)
+%{ temp=($basetype)lua_tointeger(L,$input); $1=%}
+%typemap(out) TYPE
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+%typemap(out) const TYPE&
+%{ lua_pushinteger(L, (lua_Integer) $1); SWIG_arg++;%}
+
+// Pointer and reference mapping
+%typemap(in,checkfn="lua_isinteger") TYPE *INPUT($*ltype temp), TYPE ($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+%typemap(in, numinputs=0) TYPE *OUTPUT ($*ltype temp)
+%{ $1 =  %}
+%typemap(argout) TYPE *OUTPUT
+%{  lua_pushinteger(L, (lua_Integer) *$1); SWIG_arg++;%}
+%typemap(in) TYPE *INOUT = TYPE *INPUT;
+%typemap(argout) TYPE *INOUT = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *OUTPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+%typemap(in) TYPE  = TYPE *INPUT;
+%typemap(argout) TYPE  = TYPE *OUTPUT;
+// const version (the $*ltype is the basic number without ptr or const's)
+%typemap(in,checkfn="lua_isinteger") const TYPE *INPUT($*ltype temp)
+%{ temp = ($*ltype)lua_tointeger(L,$input);
+   $1 =  %}
+
+%enddef // LLDB_NUMBER_TYPEMAP
+
+LLDB_NUMBER_TYPEMAP(unsigned char);
+LLDB_NUMBER_TYPEMAP(signed char);
+LLDB_NUMBER_TYPEMAP(short);
+LLDB_NUMBER_TYPEMAP(unsigned short);
+LLDB_NUMBER_TYPEMAP(signed short);
+LLDB_NUMBER_TYPEMAP(int);
+LLDB_NUMBER_TYPEMAP(unsigned int);
+LLDB_NUMBER_TYPEMAP(signed int);
+LLDB_NUMBER_TYPEMAP(long);
+LLDB_NUMBER_TYPEMAP(unsigned long);
+LLDB_NUMBER_TYPEMAP(signed long);
+LLDB_NUMBER_TYPEMAP(long long);
+LLDB_NUMBER_TYPEMAP(unsigned long long);
+LLDB_NUMBER_TYPEMAP(signed long long);
+
+%apply unsigned long { size_t };
+%apply const unsigned long & { const size_t & };
+%apply long { ssize_t };
+%apply const long & { const ssize_t & };
+
+//===--===//
+
+/* Typemap definitions to allow SWIG to properly handle char buffer. */
+
+// typemap for a char buffer
+%typemap(in) (char *dst, size_t dst_len) {
+   $2 = luaL_checkinteger(L, $input);
+   if ($2 <= 0) {
+   return luaL_error(L, "Positive integer expected");
+   }
+   $1 = (char *) malloc($2);
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+// Return the char buffer.  Discarding any previous return result
+%typemap(argout) (char *dst, size_t dst_len) {
+   lua_pop(L, 1); // Blow away the previous result
+   if ($result == 0) {
+  lua_pushliteral(L, "");
+   } else {
+  lua_pushlstring(L, (const char *)$1, $result);
+   }
+   free($1);
+   // SWIG_arg was already incremented
+}
+
+// SBProcess::ReadCStringFromMemory() uses a void*, but needs to be treated
+// as char data instead of byte data.
+%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+
+//===--===//
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2511222 , @JDevlieghere 
wrote:

> 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::string`s (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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

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::string`s (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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2505676 , @JDevlieghere 
wrote:

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D94937#2505676 , @JDevlieghere 
wrote:

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 317439.
tammela added a comment.

Removing spurious line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/interface/SBStructuredData.i
  lldb/examples/python/in_call_stack.py
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/source/API/SBStructuredData.cpp
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/target/stop-hooks/stop_hook.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bktptcmd.py
  lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
  
lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
  lldb/test/API/functionalities/step_scripted/Steps.py
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -100,7 +100,7 @@
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
 
 # Check API returning 'string' value
-output = string_struct.GetStringValue(25)
+output = string_struct.GetStringValue()
 if not "STRING" in output:
 self.fail("wrong output: " + output)
 
@@ -131,7 +131,7 @@
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = int_struct.GetStringValue()
 if output:
 self.fail(
 "Valid string " +
@@ -192,7 +192,7 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "23":
 self.fail("wrong output: " + str(output))
 
@@ -201,6 +201,6 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "arr":
 self.fail("wrong output: " + str(output))
Index: lldb/test/API/functionalities/step_scripted/Steps.py
===
--- lldb/test/API/functionalities/step_scripted/Steps.py
+++ lldb/test/API/functionalities/step_scripted/Steps.py
@@ -45,7 +45,7 @@
 
 if not func_entry.IsValid():
 print("Did not get a valid entry for variable_name")
-func_name = func_entry.GetStringValue(100)
+func_name = func_entry.GetStringValue()
 
 self.value = self.frame.FindVariable(func_name)
 if self.value.GetError().Fail():
@@ -88,7 +88,7 @@
 
 def __init__(self, thread_plan, args_data, dict):
 self.thread_plan = thread_plan
-self.key = args_data.GetValueForKey("token").GetStringValue(1000)
+self.key = args_data.GetValueForKey("token").GetStringValue()
 
 def should_stop(self, event):
 self.thread_plan.SetPlanComplete(True)
Index: lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
===
--- lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
+++ lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
@@ -407,7 +407,7 @@
 
 for idx in range(0, orig_keys.GetSize()):
 key = orig_keys.GetStringAtIndex(idx)
-copy_value = copy_extra_args.GetValueForKey(key).GetStringValue(100)
+copy_value = copy_extra_args.GetValueForKey(key).GetStringValue()
 
 if key == "first_arg":
 self.assertEqual(copy_value, "first_value")
Index: lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
===
--- lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
+++ lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
@@ -15,7 +15,7 @@
   sym_name = "not_a_real_function_name"
   sym_item = self.extra_args.GetValueForKey("symbol")
   if sym_item.IsValid():
-  sym_name = sym_item.GetStringValue(1000)
+  sym_name = sym_item.GetStringValue()
   else:
   print("Didn't have 

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94937

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94937: [lldb] change SBStructuredData GetStringValue signature

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch changes the GetStringValue method on the SBStructuredData
class to return a `const char *`.

This change allows the use of the method on the Lua interpreter.
The auto-generated Lua binding can't handle `char *` arguments
properly. That's because it forces the method call with a `char *` like
Lua object.

This means that to retrieve the string value an operation would need to write 
to an
internal Lua string, instead of using the Lua C API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94937

Files:
  lldb/bindings/interface/SBStructuredData.i
  lldb/examples/python/in_call_stack.py
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/source/API/SBStructuredData.cpp
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/target/stop-hooks/stop_hook.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/bktptcmd.py
  lldb/test/API/functionalities/breakpoint/scripted_bkpt/resolver.py
  
lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
  lldb/test/API/functionalities/step_scripted/Steps.py
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test
@@ -5,6 +5,7 @@
 script
 function abc(a, b, c, ...)
 print(c)
+if c then print(c:GetValueForKey("foo"):GetStringValue()) end
 end
 quit
 breakpoint command add -s lua -F abc
Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -100,7 +100,7 @@
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
 
 # Check API returning 'string' value
-output = string_struct.GetStringValue(25)
+output = string_struct.GetStringValue()
 if not "STRING" in output:
 self.fail("wrong output: " + output)
 
@@ -131,7 +131,7 @@
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = int_struct.GetStringValue()
 if output:
 self.fail(
 "Valid string " +
@@ -192,7 +192,7 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "23":
 self.fail("wrong output: " + str(output))
 
@@ -201,6 +201,6 @@
 self.fail("A valid object should have been returned")
 if not string_struct.GetType() == lldb.eStructuredDataTypeString:
 self.fail("Wrong type returned: " + str(string_struct.GetType()))
-output = string_struct.GetStringValue(5)
+output = string_struct.GetStringValue()
 if not output == "arr":
 self.fail("wrong output: " + str(output))
Index: lldb/test/API/functionalities/step_scripted/Steps.py
===
--- lldb/test/API/functionalities/step_scripted/Steps.py
+++ lldb/test/API/functionalities/step_scripted/Steps.py
@@ -45,7 +45,7 @@
 
 if not func_entry.IsValid():
 print("Did not get a valid entry for variable_name")
-func_name = func_entry.GetStringValue(100)
+func_name = func_entry.GetStringValue()
 
 self.value = self.frame.FindVariable(func_name)
 if self.value.GetError().Fail():
@@ -88,7 +88,7 @@
 
 def __init__(self, thread_plan, args_data, dict):
 self.thread_plan = thread_plan
-self.key = args_data.GetValueForKey("token").GetStringValue(1000)
+self.key = args_data.GetValueForKey("token").GetStringValue()
 
 def should_stop(self, event):
 self.thread_plan.SetPlanComplete(True)
Index: lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
===
--- lldb/test/API/functionalities/breakpoint/serialize/TestBreakpointSerialization.py
+++