[Lldb-commits] [PATCH] D117707: [lldb] [Platform] Support synthesizing siginfo_t

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: bulbazord.
labath added a comment.

Adding Alex for the plugin dependency aspect. I don't think this dep is wrong (it's pretty hard to pretend we don't at least have a C language), but he may want to say something about this.

The patch itself is somewhat repetitive, but otherwise seems ok.

I am a bit curious about the `__lldb` prefix. I'm wondering if there's a way to avoid having it, while still maintaining isolation from any type with the same name coming from the debug info. I'm thinking about putting it in some kind of an anonymous namespace or something... Does anyone know if that works?



================
Comment at: lldb/source/API/SBPlatform.cpp:672
+
+  assert(target_sp);
+  return SBType();
----------------
??


================
Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:324
+
+  // TODO: do we actually care about sparc here? lldb doesn't seem to have
+  // any sparc support
----------------
we don't


================
Comment at: lldb/unittests/Platform/PlatformSiginfoTest.cpp:83
+  void InitializeSiginfo(const std::string &triple) {
+    ArchSpec arch(triple.c_str());
+
----------------
drop c_str()


================
Comment at: lldb/unittests/Platform/PlatformSiginfoTest.cpp:109
+
+    siginfo_type = platform_sp->GetSiginfoType(*target_sp);
+  }
----------------
Other platform APIs seem to take an ArchSpec or even a Triple, so doing that would be more consistent *and* make your job here easier.


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

https://reviews.llvm.org/D117707