[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/include/lldb/API/SBScriptObject.h:47
+private:
+  std::shared_ptr<lldb_private::ScriptObject> m_opaque_sp;
+};
----------------
Does this need to be a shared pointer as opposed to a unique pointer? If so please add a comment why. 


================
Comment at: lldb/include/lldb/Utility/ScriptObject.h:9
+
+#ifndef LLDB_INTERPRETER_SCRIPTOBJECT_H
+#define LLDB_INTERPRETER_SCRIPTOBJECT_H
----------------
Not sure if this really belongs in Utility. What about putting it under `Plugins/ScriptInterpreter/` or `Interpreter/` next  to `ScriptInterpreter.h`. The header guard seems to suggest that it was there at some point? 


================
Comment at: lldb/source/API/SBScriptObject.cpp:46
+  LLDB_INSTRUMENT_VA(this);
+  return this->operator bool();
+}
----------------
Missing newline.


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

https://reviews.llvm.org/D155161