Hui added a comment. In D56232#1349407 <https://reviews.llvm.org/D56232#1349407>, @labath wrote:
> In D56232#1348514 <https://reviews.llvm.org/D56232#1348514>, @zturner wrote: > > > Is that even necessary? If a platform is not remote aware, `IsHost()` will > > always just return `true` by definition. So we could put all of this in > > the existing `Platform` base class. > > > I remember looking at this a while a go and concluding against it, but i'm > not sure if it was impossible of just I didn't like the result. > > The issue here is that PlatformWindows and PlatformPosix already have a > m_remote_platform member (which normally is an instance of > PlatformRemoteGDBServer). To move the common class into the base one, we'd > need to move this member too. That would mean that any platform has a > "remote" member, even those that already are "remote". That sounds a bit > weird. Yes. I think the thing is the existing design makes PlatformWindows plugin play dual roles: a "host" and "remote-windows" platform which simplily is a pass-through to PlatformRemoteGDBServer. Not quite sure about such intent of design. Maybe to avoid creating new plugin for a remote platform or just to simplifying the init/release in plugin manager. To break them apart, adding back the plugin, maybe PlatformRemoteWindows PlatformWindows: host implementation only. Most common parts will go to the platform base. PlatformRemoteWindows: for communicating with remote only. Could just be RemoteAwarePlatform Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56232/new/ https://reviews.llvm.org/D56232 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
