[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

Fri Jun 21 08:46:03 PDT 2019

labath added inline comments.

================
Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:31
+class NativeProcessWindows : public NativeProcessProtocol,
+                             public ProcessDebugger {
+
----------------
Hui wrote:
> labath wrote:
> > I'm not sure this multiple inheritance is really helping anything here. While looking through the code i kept wondering "why doesn't this function just do the job itself instead of delegating to somebody else" only to have to remind myself that the function it is delegating to is actually a different object, which does not know anything about the bigger picture.
> > 
> > If using composition here wouldn't introduce additional complexities/code (which it doesn't look like it would), I think it might be better to do that instead.
> Do you mean to move what is in ProcessDebugger into nativeprocess instead?  
No, just instead of inheriting from a ProcessDebugger, making it a member variable instead. Unless there are reasons which make that hard/impossible...

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

https://reviews.llvm.org/D63165