[Lldb-commits] [PATCH] D72377: [ldlb/SWIG] Refactor extensions to be non Python-specific.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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



================
Comment at: lldb/scripts/extensions.swig:4
+  const char *lldb::SBTarget::__str__ (){
+    lldb::SBStream stream;
+    $self->GetDescription (stream, lldb::eDescriptionLevelBrief);
----------------
JDevlieghere wrote:
> of course this should be factored out and reused across different SB classes. 
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.


================
Comment at: lldb/scripts/extensions.swig:9
+    size_t desc_len = stream.GetSize();
+    if (desc_len > 0 && (desc[desc_len-1] == '\n' || desc[desc_len-1] == '\r'))
+      --desc_len;
----------------
JDevlieghere wrote:
> 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. 
FWIW, I don't find it particularly bad, especially when it gets factored into a function...


================
Comment at: lldb/scripts/extensions.swig:16-17
+
+    lldb_private::ConstString str(desc, desc_len);
+    return str.GetCString();
+  }
----------------
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...


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

https://reviews.llvm.org/D72377