jingham added a comment.

Addressed review comments.



================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:80-111
+// Static Variables
+static uint32_t g_initialize_count = 0;
+
+void PlatformDarwin::Initialize() {
+  Platform::Initialize();
+
+  if (g_initialize_count++ == 0) {
----------------
JDevlieghere wrote:
> What's the point of having this? Isn't `PlatformDarwin` an abstract class 
> anyway? 
As it currently stands, you can't create a plugin setting if the plugin doesn't 
have a CreateInstance.  I wanted `settings set 
platform.plugin.darwin.ignored-exceptions`, rather than repeating the setting 
on all the concrete instances of PlatformDarwin, since this is a Darwin 
feature, so I had to add that.


================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:176-177
+  const char *ignored_exceptions = 
GetGlobalProperties().GetIgnoredExceptions();
+  if (!ignored_exceptions || ignored_exceptions[0] == '\0' )
+    return {};
+  Args ret_args;
----------------
JDevlieghere wrote:
> Unless you expect this string to contain a null byte you could turn this into 
> a StringRef and check `::empty()` instead. 
This won't contain nulls, so that part's okay.  Using a StringRef simplifies 
the check here, but in order to make up the packet I have to call 
ignored_exceptions_as_StringRef.str() in order to have something to pass to 
packet.append.  This isn't performance critical, so I don't mind the copy, but 
since we're going to end up making a std string anyway, I used std::string 
rather than StringRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125434

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

Reply via email to