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



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

Reply via email to