[lldb] move XcodeSDK's sysroot into a separate class #144396
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) ChangesThis PR introduces a new class ( This motivation for this refactor is to eventually implement Patch is 21.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144396.diff 19 Files Affected:
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 8bb55c95773bc..6bbc950effe44 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -23,6 +23,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/UUID.h"
#include "lldb/Utility/XcodeSDK.h"
+#include "lldb/Utility/XcodeSDKPath.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index 75c7f230ddf3d..e660acc66f495 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -148,7 +148,7 @@ class SymbolFile : public PluginInterface {
virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
/// Return the Xcode SDK comp_unit was compiled against.
- virtual XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) { return {}; }
+ virtual XcodeSDKPath ParseXcodeSDK(CompileUnit &comp_unit) { return {}; }
/// This function exists because SymbolFileDWARFDebugMap may extra compile
/// units which aren't exposed as "real" compile units. In every other
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index ba4a7f09afeaa..1035b58838e9a 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -65,7 +65,7 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
lldb::LanguageType
ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
- lldb_private::XcodeSDK
+ lldb_private::XcodeSDKPath
ParseXcodeSDK(lldb_private::CompileUnit &comp_unit) override;
void InitializeObject() override;
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 35ffdabf907e7..8a44291a770b3 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -28,7 +28,7 @@
#include "lldb/Utility/StructuredData.h"
#include "lldb/Utility/Timeout.h"
#include "lldb/Utility/UserIDResolver.h"
-#include "lldb/Utility/XcodeSDK.h"
+#include "lldb/Utility/XcodeSDKPath.h"
#include "lldb/lldb-private-forward.h"
#include "lldb/lldb-public.h"
@@ -442,16 +442,16 @@ class Platform : public PluginInterface {
/// Search each CU associated with the specified 'module' for
/// the SDK paths the CUs were compiled against. In the presence
/// of different SDKs, we try to pick the most appropriate one
- /// using \ref XcodeSDK::Merge.
+ /// using \ref XcodeSDKPath::Merge.
///
/// \param[in] module Module whose debug-info CUs to parse for
/// which SDK they were compiled against.
///
- /// \returns If successful, returns a pair of a parsed XcodeSDK
+ /// \returns If successful, returns a pair of a parsed XcodeSDKPath
/// object and a boolean that is 'true' if we encountered
/// a conflicting combination of SDKs when parsing the CUs
/// (e.g., a public and internal SDK).
- virtual llvm::Expected<std::pair<XcodeSDK, bool>>
+ virtual llvm::Expected<std::pair<XcodeSDKPath, bool>>
GetSDKPathFromDebugInfo(Module &module) {
return llvm::createStringError(
llvm::formatv("{0} not implemented for '{1}' platform.",
@@ -478,8 +478,9 @@ class Platform : public PluginInterface {
///
/// \param[in] unit The CU
///
- /// \returns A parsed XcodeSDK object if successful, an Error otherwise.
- virtual llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) {
+ /// \returns A parsed XcodeSDKPath object if successful, an Error otherwise.
+ virtual llvm::Expected<XcodeSDKPath>
+ GetSDKPathFromDebugInfo(CompileUnit &unit) {
return llvm::createStringError(
llvm::formatv("{0} not implemented for '{1}' platform.",
LLVM_PRETTY_FUNCTION, GetName()));
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h b/lldb/include/lldb/Utility/XcodeSDK.h
index ceb8abb8c502d..2720d0d8a44a1 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -9,7 +9,6 @@
#ifndef LLDB_UTILITY_SDK_H
#define LLDB_UTILITY_SDK_H
-#include "lldb/Utility/FileSpec.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/VersionTuple.h"
@@ -24,7 +23,6 @@ namespace lldb_private {
/// An abstraction for Xcode-style SDKs that works like \ref ArchSpec.
class XcodeSDK {
std::string m_name;
- FileSpec m_sysroot;
public:
/// Different types of Xcode SDKs.
@@ -64,10 +62,6 @@ class XcodeSDK {
/// directory component of a path one would pass to clang's -isysroot
/// 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)) {
- 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:
@@ -85,7 +79,6 @@ class XcodeSDK {
llvm::VersionTuple GetVersion() const;
Type GetType() const;
llvm::StringRef GetString() const;
- const FileSpec &GetSysroot() const;
/// Whether this Xcode SDK supports Swift.
bool SupportsSwift() const;
diff --git a/lldb/include/lldb/Utility/XcodeSDKPath.h b/lldb/include/lldb/Utility/XcodeSDKPath.h
new file mode 100644
index 0000000000000..68a2ca1d33bad
--- /dev/null
+++ b/lldb/include/lldb/Utility/XcodeSDKPath.h
@@ -0,0 +1,48 @@
+//===-- XcodeSDK.h ----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_SDK_PATH_H
+#define LLDB_UTILITY_SDK_PATH_H
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/XcodeSDK.h"
+
+namespace llvm {
+class Triple;
+}
+
+namespace lldb_private {
+
+/// An abstraction which groups an XcodeSDK with its parsed path.
+class XcodeSDKPath {
+ XcodeSDK m_sdk;
+ FileSpec m_sysroot;
+
+public:
+ /// Default constructor, constructs an empty sdk with an empty path.
+ XcodeSDKPath() = default;
+ XcodeSDKPath(XcodeSDK sdk, FileSpec sysroot)
+ : m_sdk(std::move(sdk)), m_sysroot(std::move(sysroot)) {}
+ XcodeSDKPath(std::string name, FileSpec sysroot)
+ : m_sdk(XcodeSDK(std::move(name))), m_sysroot(std::move(sysroot)) {}
+
+ bool operator==(const XcodeSDKPath &other) const;
+ bool operator!=(const XcodeSDKPath &other) const;
+
+ XcodeSDK TakeSDK() const;
+ const FileSpec &GetSysroot() const { return m_sysroot; }
+ llvm::StringRef GetString() const { return m_sdk.GetString(); }
+ XcodeSDK::Type GetType() const { return m_sdk.GetType(); }
+
+ void Merge(const XcodeSDKPath &other);
+ bool IsAppleInternalSDK() const { return m_sdk.IsAppleInternalSDK(); }
+};
+
+} // namespace lldb_private
+
+#endif
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 7aa9cae5a5614..9cc2367de3c8e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -342,7 +342,7 @@ sdkSupportsBuiltinModules(lldb_private::Target &target) {
// Use the SDK path from debug-info to find a local matching SDK directory.
auto sdk_path_or_err =
- HostInfo::GetSDKRoot(HostInfo::SDKOptions{std::move(sdk_or_err->first)});
+ HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk_or_err->first.TakeSDK()});
if (!sdk_path_or_err)
return sdk_path_or_err.takeError();
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 262a7dc731713..f271bdb4e7436 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1356,7 +1356,7 @@ llvm::Triple::OSType PlatformDarwin::GetHostOSType() {
#endif // __APPLE__
}
-llvm::Expected<std::pair<XcodeSDK, bool>>
+llvm::Expected<std::pair<XcodeSDKPath, bool>>
PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
SymbolFile *sym_file = module.GetSymbolFile();
if (!sym_file)
@@ -1367,7 +1367,7 @@ PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
bool found_public_sdk = false;
bool found_internal_sdk = false;
- XcodeSDK merged_sdk;
+ XcodeSDKPath merged_sdk_path;
for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) {
if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) {
auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp);
@@ -1375,13 +1375,13 @@ PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
found_public_sdk |= !is_internal_sdk;
found_internal_sdk |= is_internal_sdk;
- merged_sdk.Merge(cu_sdk);
+ merged_sdk_path.Merge(cu_sdk);
}
}
const bool found_mismatch = found_internal_sdk && found_public_sdk;
- return std::pair{std::move(merged_sdk), found_mismatch};
+ return std::pair{std::move(merged_sdk_path), found_mismatch};
}
llvm::Expected<std::string>
@@ -1393,23 +1393,24 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) {
llvm::formatv("Failed to parse SDK path from debug-info: {0}",
llvm::toString(sdk_or_err.takeError())));
- auto [sdk, _] = std::move(*sdk_or_err);
+ auto [sdk_path, _] = std::move(*sdk_or_err);
- if (FileSystem::Instance().Exists(sdk.GetSysroot()))
- return sdk.GetSysroot().GetPath();
+ if (FileSystem::Instance().Exists(sdk_path.GetSysroot()))
+ return sdk_path.GetSysroot().GetPath();
- auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
+ auto path_or_err =
+ HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk_path.TakeSDK()});
if (!path_or_err)
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("Error while searching for SDK (XcodeSDK '{0}'): {1}",
- sdk.GetString(),
+ sdk_path.GetString(),
llvm::toString(path_or_err.takeError())));
return path_or_err->str();
}
-llvm::Expected<XcodeSDK>
+llvm::Expected<XcodeSDKPath>
PlatformDarwin::GetSDKPathFromDebugInfo(CompileUnit &unit) {
ModuleSP module_sp = unit.CalculateSymbolContextModule();
if (!module_sp)
@@ -1434,7 +1435,7 @@ PlatformDarwin::ResolveSDKPathFromDebugInfo(CompileUnit &unit) {
auto sdk = std::move(*sdk_or_err);
- auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
+ auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk.TakeSDK()});
if (!path_or_err)
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index f8a62ceb958fe..d53dec7f47c4d 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -117,13 +117,14 @@ class PlatformDarwin : public PlatformPOSIX {
llvm::Expected<StructuredData::DictionarySP>
FetchExtendedCrashInformation(Process &process) override;
- llvm::Expected<std::pair<XcodeSDK, bool>>
+ llvm::Expected<std::pair<XcodeSDKPath, bool>>
GetSDKPathFromDebugInfo(Module &module) override;
llvm::Expected<std::string>
ResolveSDKPathFromDebugInfo(Module &module) override;
- llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) override;
+ llvm::Expected<XcodeSDKPath>
+ GetSDKPathFromDebugInfo(CompileUnit &unit) override;
llvm::Expected<std::string>
ResolveSDKPathFromDebugInfo(CompileUnit &unit) override;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 71f204c03a42a..8a06c53a82499 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -982,7 +982,7 @@ lldb::LanguageType SymbolFileDWARF::ParseLanguage(CompileUnit &comp_unit) {
return eLanguageTypeUnknown;
}
-XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
+XcodeSDKPath SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
if (!dwarf_cu)
@@ -1012,7 +1012,7 @@ XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) {
local_module_sp->RegisterXcodeSDK(sdk, sysroot);
}
- return {sdk, FileSpec{std::move(sysroot)}};
+ return {XcodeSDK(sdk), FileSpec{std::move(sysroot)}};
}
size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index d2d30d7decb16..27a5f2996b939 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -110,7 +110,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override;
- XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) override;
+ XcodeSDKPath ParseXcodeSDK(CompileUnit &comp_unit) override;
size_t ParseFunctions(CompileUnit &comp_unit) override;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index f3a940b2ee396..11371df709405 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -675,7 +675,7 @@ SymbolFileDWARFDebugMap::ParseLanguage(CompileUnit &comp_unit) {
return eLanguageTypeUnknown;
}
-XcodeSDK SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) {
+XcodeSDKPath SymbolFileDWARFDebugMap::ParseXcodeSDK(CompileUnit &comp_unit) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit);
if (oso_dwarf)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
index 35cbdbbb1692f..f363a79fe72b1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -64,7 +64,7 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
// Compile Unit function calls
lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override;
- XcodeSDK ParseXcodeSDK(CompileUnit &comp_unit) override;
+ XcodeSDKPath ParseXcodeSDK(CompileUnit &comp_unit) override;
llvm::SmallSet<lldb::LanguageType, 4>
ParseAllLanguages(CompileUnit &comp_unit) override;
size_t ParseFunctions(CompileUnit &comp_unit) override;
diff --git a/lldb/source/Symbol/SymbolFileOnDemand.cpp b/lldb/source/Symbol/SymbolFileOnDemand.cpp
index 807c2124e48d9..f886ca269d94b 100644
--- a/lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ b/lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -58,13 +58,13 @@ lldb::LanguageType SymbolFileOnDemand::ParseLanguage(CompileUnit &comp_unit) {
return m_sym_file_impl->ParseLanguage(comp_unit);
}
-XcodeSDK SymbolFileOnDemand::ParseXcodeSDK(CompileUnit &comp_unit) {
+XcodeSDKPath SymbolFileOnDemand::ParseXcodeSDK(CompileUnit &comp_unit) {
if (!m_debug_info_enabled) {
Log *log = GetLog();
LLDB_LOG(log, "[{0}] {1} is skipped", GetSymbolFileName(), __FUNCTION__);
- XcodeSDK defaultValue{};
+ XcodeSDKPath defaultValue{};
if (log) {
- XcodeSDK sdk = m_sym_file_impl->ParseXcodeSDK(comp_unit);
+ XcodeSDKPath sdk = m_sym_file_impl->ParseXcodeSDK(comp_unit);
if (!(sdk == defaultValue))
LLDB_LOG(log, "SDK {0} would return if hydrated.", sdk.GetString());
}
diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt
index 5732d977d4be4..7f08510a7e648 100644
--- a/lldb/source/Utility/CMakeLists.txt
+++ b/lldb/source/Utility/CMakeLists.txt
@@ -79,6 +79,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES
VASprintf.cpp
VMRange.cpp
XcodeSDK.cpp
+ XcodeSDKPath.cpp
ZipFile.cpp
LINK_COMPONENTS
diff --git a/lldb/source/Utility/XcodeSDK.cpp b/lldb/source/Utility/XcodeSDK.cpp
index 004b4717e315b..04df45e8c7a2a 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -142,8 +142,6 @@ XcodeSDK::Type XcodeSDK::GetType() const {
llvm::StringRef XcodeSDK::GetString() const { return m_name; }
-const FileSpec &XcodeSDK::GetSysroot() const { return m_sysroot; }
-
bool XcodeSDK::Info::operator<(const Info &other) const {
return std::tie(type, version, internal) <
std::tie(other.type, other.version, other.internal);
@@ -168,10 +166,6 @@ void XcodeSDK::Merge(const XcodeSDK &other) {
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/source/Utility/XcodeSDKPath.cpp b/lldb/source/Utility/XcodeSDKPath.cpp
new file mode 100644
index 0000000000000..e742fbe94c9b2
--- /dev/null
+++ b/lldb/source/Utility/XcodeSDKPath.cpp
@@ -0,0 +1,29 @@
+//===-- XcodeSDKPath.cpp --------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Utility/XcodeSDKPath.h"
+
+#include <string>
+
+using namespace lldb;
+using namespace lldb_private;
+
+void XcodeSDKPath::Merge(const XcodeSDKPath &other) {
+ m_sdk.Merge(other.m_sdk);
+
+ // We changed the SDK name. Adjust the sysroot accordingly.
+ auto name = m_sdk.GetString();
+ if (m_sysroot && m_sysroot.GetFilename().GetStringRef() != name)
+ m_sysroot.SetFilename(name);
+}
+
+XcodeSDK XcodeSDKPath::TakeSDK() const { return std::move(m_sdk); }
+
+bool XcodeSDKPath::operator==(const XcodeSDKPath &other) const {
+ return m_sdk == other.m_sdk && m_sysroot == other.m_sysroot;
+}
diff --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index 50c37dcd4568e..772f901a3ffe3 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -116,7 +116,7 @@ TEST_F(XcodeSDKModuleTests, TestModuleGetXcodeSDK) {
SymbolFileDWARF &sym_file = dwarf_cu->GetSymbolFileDWARF();
CompUnitSP comp_unit = sym_file.GetCompileUnitAtIndex(0);
ASSERT_TRUE(static_cast<bool>(comp_unit.get()));
- XcodeSDK sdk = sym_file.ParseXcodeSDK(*comp_unit);
+ XcodeSDKPath sdk = sym_file.ParseXcodeSDK(*comp_unit);
ASSERT_EQ(sdk.GetType(), XcodeSDK::Type::MacOSX);
}
diff --git a/lldb/unittests/Utility/XcodeSDKTest.cpp b/lldb/unittests/Utility/XcodeSDKTest.cpp
index 4db6a50fcf860..c0e258ab5b2db 100644
--- a/lldb/unittests/Utility/XcodeSDKTest.cpp
+++ b/lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -10,6 +10,7 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/XcodeSDK.h"
+#include "lldb/Utility/XcodeSDKPath.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/TargetParser/Triple.h"
@@ -35,16 +36,14 @@ TEST(XcodeSDKTest, ParseTest) {
EXPECT_EQ(XcodeSDK("MacOSX10.15.4.sdk").GetVersion(), llvm::VersionTuple(10, 15));
EXPECT_EQ(XcodeSDK("MacOSX.sdk").IsAppleInternalSDK(), false);
EXPECT_EQ(
- XcodeSDK("MacOSX.sdk", FileSpec{"/Path/To/MacOSX.sdk"}).GetSysroot(),
+ XcodeSDKPath("MacOSX.sdk", FileSpec{"/Path/To/MacOSX.sdk"}).GetSysroot(),
FileSpec("/Path/To/MacOSX.sdk"));
EXPECT_EQ(XcodeSDK("MacOSX10.15.Internal.sdk").GetType(), XcodeSDK::MacOSX);
EXPECT_EQ(XcodeSDK("MacOSX10.15.Interna...
[truncated]
|
|
Looks plausible. Out of curiosity, why is this needed to implement |
Not yet, the original motivation is this issue on the Swift forums and more specifically this comment: https://forums.swift.org/t/lldb-crashes-during-vs-code-debug-session-on-windows-10/79902/10
|
There was a problem hiding this comment.
I'm not sure I understand the need for the separate class. What's present in the XcodeSDK class that prevents you from using it on PlatformWindows? Given that the current class is part of Utility, it shouldn't depend on anything Darwin specific. If we need to make this class behave differently depending on the platform, then maybe it's time to make this a generic "SDK" plugin with different implementations depending on the platform?
| bool operator==(const XcodeSDKPath &other) const; | ||
| bool operator!=(const XcodeSDKPath &other) const; | ||
|
|
||
| XcodeSDK TakeSDK() const; |
There was a problem hiding this comment.
Suggested change
| XcodeSDK TakeSDK() const; | |
| XcodeSDK GetSDK() const; |
I suggested this change. My thinking was that we should separate out an SDK specification from a resolved local SDK and that it would clarify APIs if the two were separate types, so we know if we deal with a resolved absolute path, or abstract specification like "any iOS 18.5 SDK". |
| @@ -0,0 +1,48 @@ | |||
| //===-- XcodeSDK.h ----------------------------------------------*- C++ -*-===// | |||
There was a problem hiding this comment.
I think I would just put this into the existing XcodeSDK.h header, since it's so closely related.
| @@ -982,7 +982,7 @@ lldb::LanguageType SymbolFileDWARF::ParseLanguage(CompileUnit &comp_unit) { | |||
| return eLanguageTypeUnknown; | |||
| } | |||
|
|
|||
| XcodeSDK SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { | |||
| XcodeSDKPath SymbolFileDWARF::ParseXcodeSDK(CompileUnit &comp_unit) { | |||
There was a problem hiding this comment.
Thinking out aloud: I was just going to write: "wouldn't it be better if this function returned an abstract SDK specification?" but then I realized that this is the point where we can read an absolute oath to the sysroot out of DWARF, so it needs to be am absolute path.
There was a problem hiding this comment.
Yes, I think there is a confusing notion of "the path parsed from DWARF" vs "the path of the SDK on the system we are debugging this on".
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a new class (
XcodeSDKPath) which holds both anXcodeSDKand asysrootfor that SDK.This motivation for this refactor is to eventually implement
GetSDKPathFromDebugInfoonPlatformWindows.