JDevlieghere added inline comments.
================
Comment at: lldb/source/Plugins/Platform/CMakeLists.txt:9
add_subdirectory(POSIX)
+add_subdirectory(scripted)
add_subdirectory(QemuUser)
----------------
================
Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.cpp:208
+ ProcessInstanceInfoList &proc_infos) {
+ CheckInterpreterAndScriptObject();
+ StructuredData::DictionarySP dict_sp = GetInterface().ListProcesses();
----------------
What's your intention by calling this function? In a release build this is
still going to crash. Is there a way to initialize the platform with an invalid
`m_script_object_sp` or `m_interpreter`? If so then we should have proper error
handling. If not then these can be regular asserts.
================
Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h:49-52
+ lldb_private::Target *target, // Can be nullptr, if
+ // nullptr create a new
+ // target, else use
+ // existing one
----------------
This should be a Doxygen comment.
================
Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h:70-74
+ void CheckInterpreterAndScriptObject() const;
+ ScriptedPlatformInterface &GetInterface() const;
+ llvm::Expected<ProcessInstanceInfo>
+ ParseProcessInfo(StructuredData::Dictionary &dict, lldb::pid_t pid) const;
+ static bool IsScriptLanguageSupported(lldb::ScriptLanguage language);
----------------
This is hard to parse, do they really need to be grouped together? I'd add a
newline between them like the public functions.
================
Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h:76
+
+ // Member variables.
+ const ScriptedMetadata *m_scripted_metadata = nullptr;
----------------
Useless comment
================
Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.h:80
+ lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
+ //@}
+};
----------------
Missing group opener (`@{`), but you can just remove it if you do the same for
line 76
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139252/new/
https://reviews.llvm.org/D139252
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
