labath added a comment. Ok, I think we have converged onto something here. I just think it would be good to rename some functions/variables to make it clear they are working with an xcode sdk, and not some generic entity.
In D76471#1955908 <https://reviews.llvm.org/D76471#1955908>, @aprantl wrote: > Great! I think the current version of the patch reflects all of that, with > only one difference — something that I actually overlooked in the previous > version. We now have Platform that provides the interface to provide an SDK, > and will forward the question to Host. Each Host can define its own > mechanisms for doing so. So far I have only provided an implementation for > macOS, which runs `xcrun`. What I can't do is remove the SDK parameter from > the interface in platform, because Xcode may contain differently versioned > SDKs (e.g., a MacOSX10.14.sdk and a MacOSX10.15.sdk) plus some wrinkles that > are only relevant when you are developing the OS itself. That said, even > without removing the SDK parameter from GetSDKPath(), we can still implement > HostInfoLinux::GetXcodeSDK() for hypothetical Linux -> macOS cross-debugger > you mentioned above, so I don't think the extra parameter tarnishes the > design. Yeah, it seems acceptable. The "sdk type" argument still seems somewhat redundant. Theoretically we could pass in just the triple component out the xcodesdk object, but that would just mean the individual platforms would have to reconstruct the XcodeSDK object, which is already present in the caller, so it's not ideal either... ================ Comment at: lldb/include/lldb/Core/Module.h:517 + /// \param sysroot will be added to the path remapping dictionary. + void RegisterSDK(llvm::StringRef sdk, llvm::StringRef sysroot); + ---------------- labath wrote: > RegisterXCodeSDK? ping ================ Comment at: lldb/include/lldb/Core/Module.h:983 + /// The SDK this module was compiled with. + XcodeSDK m_sdk; + ---------------- labath wrote: > m_xcode_sdk ping? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76471/new/ https://reviews.llvm.org/D76471 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
