[Lldb-commits] [lldb] f4be9ff - [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings
Alex Langford via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 14 10:00:14 PDT 2023
Author: Alex Langford
Date: 2023-06-14T09:59:58-07:00
New Revision: f4be9ff6458fe6401acaf9239865e077141dd8bc
URL: https://github.com/llvm/llvm-project/commit/f4be9ff6458fe6401acaf9239865e077141dd8bc
DIFF: https://github.com/llvm/llvm-project/commit/f4be9ff6458fe6401acaf9239865e077141dd8bc.diff
LOG: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings
These don't need to be ConstStrings. They don't really benefit much from
deduplication and comparing them isn't on a hot path, so they don't
really benefit much from quick comparisons.
Differential Revision: https://reviews.llvm.org/D152331
Added:
Modified:
lldb/include/lldb/Interpreter/OptionGroupPlatform.h
lldb/include/lldb/Target/Platform.h
lldb/source/API/SBPlatform.cpp
lldb/source/Interpreter/OptionGroupPlatform.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Target/Platform.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Interpreter/OptionGroupPlatform.h b/lldb/include/lldb/Interpreter/OptionGroupPlatform.h
index fed2791a61309..2ffd44f0035de 100644
--- a/lldb/include/lldb/Interpreter/OptionGroupPlatform.h
+++ b/lldb/include/lldb/Interpreter/OptionGroupPlatform.h
@@ -47,22 +47,24 @@ class OptionGroupPlatform : public OptionGroup {
m_platform_name.clear();
}
- ConstString GetSDKRootDirectory() const { return m_sdk_sysroot; }
+ const std::string &GetSDKRootDirectory() const { return m_sdk_sysroot; }
- void SetSDKRootDirectory(ConstString sdk_root_directory) {
- m_sdk_sysroot = sdk_root_directory;
+ void SetSDKRootDirectory(std::string sdk_root_directory) {
+ m_sdk_sysroot = std::move(sdk_root_directory);
}
- ConstString GetSDKBuild() const { return m_sdk_build; }
+ const std::string &GetSDKBuild() const { return m_sdk_build; }
- void SetSDKBuild(ConstString sdk_build) { m_sdk_build = sdk_build; }
+ void SetSDKBuild(std::string sdk_build) {
+ m_sdk_build = std::move(sdk_build);
+ }
bool PlatformMatches(const lldb::PlatformSP &platform_sp) const;
protected:
std::string m_platform_name;
- ConstString m_sdk_sysroot;
- ConstString m_sdk_build;
+ std::string m_sdk_sysroot;
+ std::string m_sdk_build;
llvm::VersionTuple m_os_version;
bool m_include_platform_option;
};
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 08e47cc132473..b556e1aa3dd2e 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -448,13 +448,15 @@ class Platform : public PluginInterface {
// Used for column widths
size_t GetMaxGroupIDNameLength() const { return m_max_gid_name_len; }
- ConstString GetSDKRootDirectory() const { return m_sdk_sysroot; }
+ const std::string &GetSDKRootDirectory() const { return m_sdk_sysroot; }
- void SetSDKRootDirectory(ConstString dir) { m_sdk_sysroot = dir; }
+ void SetSDKRootDirectory(std::string dir) { m_sdk_sysroot = std::move(dir); }
- ConstString GetSDKBuild() const { return m_sdk_build; }
+ const std::string &GetSDKBuild() const { return m_sdk_build; }
- void SetSDKBuild(ConstString sdk_build) { m_sdk_build = sdk_build; }
+ void SetSDKBuild(std::string sdk_build) {
+ m_sdk_build = std::move(sdk_build);
+ }
// Override this to return true if your platform supports Clang modules. You
// may also need to override AddClangModuleCompilationOptions to pass the
@@ -900,9 +902,9 @@ class Platform : public PluginInterface {
// the once we call HostInfo::GetOSVersion().
bool m_os_version_set_while_connected;
bool m_system_arch_set_while_connected;
- ConstString
+ std::string
m_sdk_sysroot; // the root location of where the SDK files are all located
- ConstString m_sdk_build;
+ std::string m_sdk_build;
FileSpec m_working_dir; // The working directory which is used when installing
// modules that have no install path set
std::string m_remote_url;
diff --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index c678edcd810da..12461f3713eca 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -488,7 +488,7 @@ uint32_t SBPlatform::GetOSUpdateVersion() {
void SBPlatform::SetSDKRoot(const char *sysroot) {
LLDB_INSTRUMENT_VA(this, sysroot);
if (PlatformSP platform_sp = GetSP())
- platform_sp->SetSDKRootDirectory(ConstString(sysroot));
+ platform_sp->SetSDKRootDirectory(sysroot);
}
SBError SBPlatform::Get(SBFileSpec &src, SBFileSpec &dst) {
diff --git a/lldb/source/Interpreter/OptionGroupPlatform.cpp b/lldb/source/Interpreter/OptionGroupPlatform.cpp
index 60107eb288069..9928a5cda03e2 100644
--- a/lldb/source/Interpreter/OptionGroupPlatform.cpp
+++ b/lldb/source/Interpreter/OptionGroupPlatform.cpp
@@ -50,10 +50,10 @@ PlatformSP OptionGroupPlatform::CreatePlatformWithOptions(
if (!m_os_version.empty())
platform_sp->SetOSVersion(m_os_version);
- if (m_sdk_sysroot)
+ if (!m_sdk_sysroot.empty())
platform_sp->SetSDKRootDirectory(m_sdk_sysroot);
- if (m_sdk_build)
+ if (!m_sdk_build.empty())
platform_sp->SetSDKBuild(m_sdk_build);
}
@@ -63,8 +63,8 @@ PlatformSP OptionGroupPlatform::CreatePlatformWithOptions(
void OptionGroupPlatform::OptionParsingStarting(
ExecutionContext *execution_context) {
m_platform_name.clear();
- m_sdk_sysroot.Clear();
- m_sdk_build.Clear();
+ m_sdk_sysroot.clear();
+ m_sdk_build.clear();
m_os_version = llvm::VersionTuple();
}
@@ -103,7 +103,7 @@ OptionGroupPlatform::SetOptionValue(uint32_t option_idx,
switch (short_option) {
case 'p':
- m_platform_name.assign(std::string(option_arg));
+ m_platform_name.assign(option_arg.str());
break;
case 'v':
@@ -113,11 +113,11 @@ OptionGroupPlatform::SetOptionValue(uint32_t option_idx,
break;
case 'b':
- m_sdk_build.SetString(option_arg);
+ m_sdk_build.assign(option_arg.str());
break;
case 'S':
- m_sdk_sysroot.SetString(option_arg);
+ m_sdk_sysroot.assign(option_arg.str());
break;
default:
@@ -128,21 +128,21 @@ OptionGroupPlatform::SetOptionValue(uint32_t option_idx,
bool OptionGroupPlatform::PlatformMatches(
const lldb::PlatformSP &platform_sp) const {
- if (platform_sp) {
- if (!m_platform_name.empty()) {
- if (platform_sp->GetName() != m_platform_name)
- return false;
- }
+ if (!platform_sp)
+ return false;
- if (m_sdk_build && m_sdk_build != platform_sp->GetSDKBuild())
- return false;
+ if (!m_platform_name.empty() && platform_sp->GetName() != m_platform_name)
+ return false;
- if (m_sdk_sysroot && m_sdk_sysroot != platform_sp->GetSDKRootDirectory())
- return false;
+ if (!m_sdk_build.empty() && platform_sp->GetSDKBuild() != m_sdk_build)
+ return false;
- if (!m_os_version.empty() && m_os_version != platform_sp->GetOSVersion())
- return false;
- return true;
- }
- return false;
+ if (!m_sdk_sysroot.empty() &&
+ platform_sp->GetSDKRootDirectory() != m_sdk_sysroot)
+ return false;
+
+ if (!m_os_version.empty() && platform_sp->GetOSVersion() != m_os_version)
+ return false;
+
+ return true;
}
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
index d8007b4f4727f..52777909a1f82 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -34,8 +34,8 @@ bool PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded() {
std::lock_guard<std::mutex> guard(m_sdk_dir_mutex);
if (m_sdk_directory_infos.empty()) {
// A --sysroot option was supplied - add it to our list of SDKs to check
- if (m_sdk_sysroot) {
- FileSpec sdk_sysroot_fspec(m_sdk_sysroot.GetCString());
+ if (!m_sdk_sysroot.empty()) {
+ FileSpec sdk_sysroot_fspec(m_sdk_sysroot.c_str());
FileSystem::Instance().Resolve(sdk_sysroot_fspec);
const SDKDirectoryInfo sdk_sysroot_directory_info(sdk_sysroot_fspec);
m_sdk_directory_infos.push_back(sdk_sysroot_directory_info);
@@ -43,7 +43,7 @@ bool PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded() {
LLDB_LOGF(log,
"PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded added "
"--sysroot SDK directory %s",
- m_sdk_sysroot.GetCString());
+ m_sdk_sysroot.c_str());
}
return true;
}
@@ -152,20 +152,20 @@ PlatformDarwinDevice::GetSDKDirectoryForCurrentOSVersion() {
std::vector<bool> check_sdk_info(num_sdk_infos, true);
// Prefer the user SDK build string.
- ConstString build = GetSDKBuild();
+ std::string build = GetSDKBuild();
// Fall back to the platform's build string.
- if (!build) {
- if (std::optional<std::string> os_build_str = GetOSBuildString()) {
- build = ConstString(*os_build_str);
- }
+ if (build.empty()) {
+ if (std::optional<std::string> os_build_str = GetOSBuildString())
+ build.assign(*os_build_str);
}
// If we have a build string, only check platforms for which the build
// string matches.
- if (build) {
+ if (!build.empty()) {
for (i = 0; i < num_sdk_infos; ++i)
- check_sdk_info[i] = m_sdk_directory_infos[i].build == build;
+ check_sdk_info[i] = m_sdk_directory_infos[i].build.GetStringRef() ==
+ llvm::StringRef(build);
}
// If we are connected we can find the version of the OS the platform us
@@ -201,7 +201,7 @@ PlatformDarwinDevice::GetSDKDirectoryForCurrentOSVersion() {
}
}
}
- } else if (build) {
+ } else if (!build.empty()) {
// No version, just a build number, return the first one that matches.
for (i = 0; i < num_sdk_infos; ++i)
if (check_sdk_info[i])
@@ -248,8 +248,8 @@ const char *PlatformDarwinDevice::GetDeviceSupportDirectory() {
}
const char *PlatformDarwinDevice::GetDeviceSupportDirectoryForOSVersion() {
- if (m_sdk_sysroot)
- return m_sdk_sysroot.GetCString();
+ if (!m_sdk_sysroot.empty())
+ return m_sdk_sysroot.c_str();
if (m_device_support_directory_for_os_version.empty()) {
const PlatformDarwinDevice::SDKDirectoryInfo *sdk_dir_info =
diff --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
index 6e94ab51354c7..72a039d188727 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -191,8 +191,8 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
launch_info.SetArguments(args, true);
Environment emulator_env = Host::GetEnvironment();
- if (ConstString sysroot = GetSDKRootDirectory())
- emulator_env["QEMU_LD_PREFIX"] = sysroot.GetStringRef().str();
+ if (const std::string &sysroot = GetSDKRootDirectory(); !sysroot.empty())
+ emulator_env["QEMU_LD_PREFIX"] = sysroot;
for (const auto &KV : GetGlobalProperties().GetEmulatorEnvVars())
emulator_env[KV.first()] = KV.second;
launch_info.GetEnvironment() = ComputeLaunchEnvironment(
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index f81f2353d867f..2590197c7149b 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -210,11 +210,10 @@ Status Platform::GetSharedModule(
Status error(eErrorTypeGeneric);
ModuleSpec resolved_spec;
// Check if we have sysroot set.
- if (m_sdk_sysroot) {
+ if (!m_sdk_sysroot.empty()) {
// Prepend sysroot to module spec.
resolved_spec = spec;
- resolved_spec.GetFileSpec().PrependPathComponent(
- m_sdk_sysroot.GetStringRef());
+ resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
// Try to get shared module with resolved spec.
error = ModuleList::GetSharedModule(resolved_spec, module_sp,
module_search_paths_ptr, old_modules,
@@ -312,9 +311,9 @@ void Platform::GetStatus(Stream &strm) {
strm.Printf(" Connected: %s\n", is_connected ? "yes" : "no");
}
- if (GetSDKRootDirectory()) {
- strm.Format(" Sysroot: {0}\n", GetSDKRootDirectory());
- }
+ if (const std::string &sdk_root = GetSDKRootDirectory(); !sdk_root.empty())
+ strm.Format(" Sysroot: {0}\n", sdk_root);
+
if (GetWorkingDirectory()) {
strm.Printf("WorkingDir: %s\n", GetWorkingDirectory().GetPath().c_str());
}
More information about the lldb-commits
mailing list