[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The vscode change seems fine to me, but I don't consider myself an authority on that.

In D134333#3812653 <https://reviews.llvm.org/D134333#3812653>, @jingham wrote:

> In D134333#3812448 <https://reviews.llvm.org/D134333#3812448>, @clayborg wrote:
>
>> I just did a search through our test sources and we use GetError().GetCString() 34 times in our test suites python files. So the SBError::GetCString() is being misused a lot like this already and is probably making some tests flaky. I would highly recommend we fix SBError::GetCString() to make sure things work correctly. If people are already doing this, or if they can do this with our API, then we should make our API as stable as possible for users even if it costs a bit more memory in our string pools.
>
> When we return Python strings from the SWIG'ed version of GetError().GetCString() does the Python string we make & return copy the string data, or is it just holding onto the pointer?  In C/C++ if someone returns you a char * unless explicitly specified you assume that string is only going to live as long as the object that handed it out.  But a python string is generally self-contained, so you wouldn't expect it to lose its data when the generating object goes away.  I haven't checked yet, but if this is how SWIG handles making python strings from C strings, then this wouldn't be an error in the testsuite, only in C++ code.  If that's not how it works, that might be a good way to finesse the issue.

The python version is fine because swig SBError::GetCString wrapper gets essentially a pointer to the SBError objects (wrapped as PyObject), and returns another PyObject with the (copied) string (also a PyObject). All the lifetime management happens outside of that function.



================
Comment at: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:48-53
+                    contains = verify_value in actual_value
+                    self.assertTrue(contains,
+                                    ('"%s" value "%s" doesn\'t contain'
+                                     ' "%s")') % (
+                                        key, actual_value,
+                                        verify_value))
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333