[Lldb-commits] [lldb] cdd560e - [lldb][XcodeSDK] Simplify logic that adjusts sysroot during XcodeSDK merging (#130640)

[via lldb-commits]

Author: Michael Buch
Date: 2025-03-11T08:13:05Z
New Revision: cdd560eead457bcc6dbb28ef88d868bc68cfd7e6

URL: https://github.com/llvm/llvm-project/commit/cdd560eead457bcc6dbb28ef88d868bc68cfd7e6
DIFF: https://github.com/llvm/llvm-project/commit/cdd560eead457bcc6dbb28ef88d868bc68cfd7e6.diff

LOG: [lldb][XcodeSDK] Simplify logic that adjusts sysroot during XcodeSDK merging (#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.

Added: 
    

Modified: 
    lldb/include/lldb/Utility/XcodeSDK.h
    lldb/source/Utility/XcodeSDK.cpp
    lldb/unittests/Utility/XcodeSDKTest.cpp

Removed: 
    


################################################################################
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..004b4717e315b 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,14 @@ 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(0, m_name.size() - 3) + std::string("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) {

diff  --git a/lldb/unittests/Utility/XcodeSDKTest.cpp b/lldb/unittests/Utility/XcodeSDKTest.cpp
index bca7f1291c425..4db6a50fcf860 100644
--- a/lldb/unittests/Utility/XcodeSDKTest.cpp
+++ b/lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -66,13 +66,14 @@ TEST(XcodeSDKTest, MergeTest) {
   empty.Merge(XcodeSDK("MacOSX10.14.Internal.sdk"));
   EXPECT_EQ(empty.GetString(), llvm::StringRef("MacOSX10.14.Internal.sdk"));
   EXPECT_FALSE(empty.GetSysroot());
-  empty.Merge(XcodeSDK("MacOSX9.5.Internal.sdk", FileSpec{"/Path/To/9.5.sdk"}));
+  empty.Merge(XcodeSDK("MacOSX9.5.Internal.sdk",
+                       FileSpec{"/Path/To/MacOSX9.5.Internal.sdk"}));
   EXPECT_FALSE(empty.GetSysroot());
-  empty.Merge(XcodeSDK("MacOSX12.5.sdk", FileSpec{"/Path/To/12.5.sdk"}));
-  EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/12.5.sdk"});
+  empty.Merge(XcodeSDK("MacOSX12.5.sdk", FileSpec{"/Path/To/MacOSX12.5.sdk"}));
+  EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/MacOSX12.5.sdk"});
   empty.Merge(XcodeSDK("MacOSX11.5.Internal.sdk",
-                       FileSpec{"/Path/To/12.5.Internal.sdk"}));
-  EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/12.5.Internal.sdk"});
+                       FileSpec{"/Path/To/MacOSX11.5.Internal.sdk"}));
+  EXPECT_EQ(empty.GetSysroot(), FileSpec{"/Path/To/MacOSX12.5.Internal.sdk"});
 }
 
 #ifndef _WIN32