[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

[Pavel Labath via Phabricator via lldb-commits]

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