Skip to content

[lldb][XcodeSDK] Simplify logic that adjusts sysroot during XcodeSDK merging #130640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

Michael137
Copy link
Member

The DW_AT_APPLE_sdk should always be equal to the filename of the DW_AT_LLVM_sysroot. We can use this property to simplify XcodeSDK::Merge to no longer manually adjust the sysroot filename. Instead we simply update the sysroot filename with merged SDK name.

This should be an NFC change.

@llvmbot
Copy link
Member

llvmbot commented Mar 10, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

The DW_AT_APPLE_sdk should always be equal to the filename of the DW_AT_LLVM_sysroot. We can use this property to simplify XcodeSDK::Merge to no longer manually adjust the sysroot filename. Instead we simply update the sysroot filename with merged SDK name.

This should be an NFC change.


Full diff: https://github.com/llvm/llvm-project/pull/130640.diff

2 Files Affected:

  • (modified) lldb/include/lldb/Utility/XcodeSDK.h (+3-1)
  • (modified) lldb/source/Utility/XcodeSDK.cpp (+5-8)
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h b/lldb/include/lldb/Utility/XcodeSDK.h
index 3a6cbb9c98f6b..ceb8abb8c502d 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -65,7 +65,9 @@ class XcodeSDK {
   /// parameter. For example, "MacOSX.10.14.sdk".
   XcodeSDK(std::string &&name) : m_name(std::move(name)) {}
   XcodeSDK(std::string name, FileSpec sysroot)
-      : m_name(std::move(name)), m_sysroot(std::move(sysroot)) {}
+      : m_name(std::move(name)), m_sysroot(std::move(sysroot)) {
+    assert(!m_sysroot || m_name == m_sysroot.GetFilename().GetStringRef());
+  }
   static XcodeSDK GetAnyMacOS() { return XcodeSDK("MacOSX.sdk"); }
 
   /// The merge function follows a strict order to maintain monotonicity:
diff --git a/lldb/source/Utility/XcodeSDK.cpp b/lldb/source/Utility/XcodeSDK.cpp
index 02cf7866e22fb..59bf9a7b9e3b4 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -155,10 +155,6 @@ bool XcodeSDK::Info::operator==(const Info &other) const {
 }
 
 void XcodeSDK::Merge(const XcodeSDK &other) {
-  auto add_internal_sdk_suffix = [](llvm::StringRef sdk) {
-    return (sdk.substr(0, sdk.size() - 3) + "Internal.sdk").str();
-  };
-
   // The "bigger" SDK always wins.
   auto l = Parse();
   auto r = other.Parse();
@@ -168,12 +164,13 @@ void XcodeSDK::Merge(const XcodeSDK &other) {
     // The Internal flag always wins.
     if (!l.internal && r.internal) {
       if (llvm::StringRef(m_name).ends_with(".sdk"))
-        m_name = add_internal_sdk_suffix(m_name);
-
-      if (m_sysroot.GetFileNameExtension() == ".sdk")
-        m_sysroot.SetFilename(add_internal_sdk_suffix(m_sysroot.GetFilename()));
+        m_name = m_name.substr(m_name.size() - 3) + "Internal.sdk";
     }
   }
+
+  // We changed the SDK name. Adjust the sysroot accordingly.
+  if (m_sysroot && m_sysroot.GetFilename().GetStringRef() != m_name)
+    m_sysroot.SetFilename(m_name);
 }
 
 std::string XcodeSDK::GetCanonicalName(XcodeSDK::Info info) {

…merging

The `DW_AT_APPLE_sdk` should always be equal to the filename of the `DW_AT_LLVM_sysroot`. We can use this property to simplify `XcodeSDK::Merge` to no longer manually adjust the sysroot filename. Instead we simply update the sysroot filename with merged SDK name.

This should be an NFC change.
@Michael137 Michael137 force-pushed the lldb/simplify-xcodesdk-sysroot branch from 678c178 to 4761db2 Compare March 10, 2025 17:31
@Michael137 Michael137 merged commit cdd560e into llvm:main Mar 11, 2025
10 checks passed
@Michael137 Michael137 deleted the lldb/simplify-xcodesdk-sysroot branch March 11, 2025 09:33
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Mar 11, 2025
…merging (llvm#130640)

The `DW_AT_APPLE_sdk` should always be equal to the filename of the
`DW_AT_LLVM_sysroot`. We can use this property to simplify
`XcodeSDK::Merge` to no longer manually adjust the sysroot filename.
Instead we simply update the sysroot filename with merged SDK name.

This should be an NFC change.

(cherry picked from commit cdd560e)
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Mar 11, 2025
…to-20240723

🍒 [lldb][XcodeSDK] Simplify logic that adjusts sysroot during XcodeSDK merging (llvm#130640)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants