[Lldb-commits] [PATCH] D78801: [LLDB] Add class ProcessWasm for WebAssembly debugging

Mon Apr 27 06:25:13 PDT 2020

labath added a comment.

In D78801#2004501 <https://reviews.llvm.org/D78801#2004501>, @paolosev wrote:

> My initial idea was to keep all the Wasm-related code as much as possible isolated in plugin classes.

While I would definitely like to see that happen, I don't think the current approach achieves that. The "IWasmProcess" is still in the wasm plugin so we still end up with a lot of "core" code depending on the wasm plugin. And if we put IWasmProcess into the "core", then it's not much different than putting the relevant APIs into the "Process" class directly (though it could introduce some grouping which might count for something). If we accept that DW_OP_WASM_location as a first-class entity, then having some core interfaces to support it would not seem unreasonable. The thing which makes that blurry is that this is a vendor extension, not an official "first class" thing.

> Now, I guess that the next steps instead would be to refactor the code to eliminate the new classes WasmProcessGDBRemote and UnwindWasm and modify existing ProcessGDBRemote and ThreadGDBRemote instead.

It may be interesting to see how that ends up looking like (maybe you could put that in a separate patch to compare), but I don't think that at this point we have chosen the right way to go forward, and we still need to discuss/think about things...

> PS, I am sorry for the late reply… this lockdown is making me a little unproductive… :-(

You replied on the next business day. I don't think this is late by any standard.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78801