[Lldb-commits] [PATCH] D139249: [lldb] Add Debugger & ScriptedMetadata reference to Platform::CreateInstance

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In addition to that, it breaks tests with ubsan. :)

I raised a similar concern in one of the other patches. I'm of the opinion that it's not worth pretending like the scripted processes (platforms, etc.) are plugins. I don't think that it can be achieved without some sort of a leakage, and I don't think it makes sense given that the part that's really interesting to have pluggable is being able to plug a different scripting language into the scripted process. I don't exactly have an opinion on this metadata argument, but it definitely looks sub-optimal.

Given this uncertainty, and the ubsan problem, maybe we can revert this for the time being?



================
Comment at: lldb/unittests/Platform/PlatformTest.cpp:90
+                   []() { Debugger::Initialize(nullptr); });
+    list = new PlatformList(*m_debugger_sp.get());
+  }
----------------
m_debugger_sp is never initialized. I guess this kinda works because the test does not actually dereference it, but UBSAN lights it up straight away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139249