This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdfd499a61c45: [lldb][NFC] avoid unnecessary roundtrips
between different string types (authored by xujuntwt95329, committed by Walter
Erquinigo <wall...@fb.com>).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112863/new/
https://reviews.llvm.org/D112863
Files:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/Module.cpp
lldb/source/Core/ModuleList.cpp
lldb/source/Interpreter/OptionValuePathMappings.cpp
lldb/source/Target/PathMappingList.cpp
lldb/unittests/Target/FindFileTest.cpp
lldb/unittests/Target/PathMappingListTest.cpp
Index: lldb/unittests/Target/PathMappingListTest.cpp
===================================================================
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -66,16 +66,16 @@
#endif
};
PathMappingList map;
- map.Append(ConstString("."), ConstString("/tmp"), false);
+ map.Append(".", "/tmp", false);
TestPathMappings(map, matches, fails);
PathMappingList map2;
- map2.Append(ConstString(""), ConstString("/tmp"), false);
+ map2.Append("", "/tmp", false);
TestPathMappings(map, matches, fails);
}
TEST(PathMappingListTest, AbsoluteTests) {
PathMappingList map;
- map.Append(ConstString("/old"), ConstString("/new"), false);
+ map.Append("/old", "/new", false);
Matches matches[] = {
{"/old", "/new"},
{"/old/", "/new"},
@@ -97,7 +97,7 @@
TEST(PathMappingListTest, RemapRoot) {
PathMappingList map;
- map.Append(ConstString("/"), ConstString("/new"), false);
+ map.Append("/", "/new", false);
Matches matches[] = {
{"/old", "/new/old"},
{"/old/", "/new/old"},
@@ -118,7 +118,7 @@
#ifndef _WIN32
TEST(PathMappingListTest, CrossPlatformTests) {
PathMappingList map;
- map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+ map.Append(R"(C:\old)", "/new", false);
Matches matches[] = {
{R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
{R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
Index: lldb/unittests/Target/FindFileTest.cpp
===================================================================
--- lldb/unittests/Target/FindFileTest.cpp
+++ lldb/unittests/Target/FindFileTest.cpp
@@ -75,8 +75,8 @@
llvm::FileRemover file_remover(FileName);
PathMappingList map;
- map.Append(ConstString("/old"), ConstString(DirName.str()), false);
- map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+ map.Append("/old", DirName.str(), false);
+ map.Append(R"(C:\foo)", DirName.str(), false);
Matches matches[] = {
{"/old", llvm::sys::path::Style::posix, DirName.c_str()},
Index: lldb/source/Target/PathMappingList.cpp
===================================================================
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -30,11 +30,11 @@
// with the raw path pair, which doesn't work anymore because the paths have
// been normalized when the debug info was loaded. So we need to store
// nomalized path pairs to ensure things match up.
- ConstString NormalizePath(ConstString path) {
- // If we use "path" to construct a FileSpec, it will normalize the path for
- // us. We then grab the string and turn it back into a ConstString.
- return ConstString(FileSpec(path.GetStringRef()).GetPath());
- }
+std::string NormalizePath(llvm::StringRef path) {
+ // If we use "path" to construct a FileSpec, it will normalize the path for
+ // us. We then grab the string.
+ return FileSpec(path).GetPath();
+}
}
// PathMappingList constructor
PathMappingList::PathMappingList() : m_pairs() {}
@@ -59,8 +59,8 @@
PathMappingList::~PathMappingList() = default;
-void PathMappingList::Append(ConstString path,
- ConstString replacement, bool notify) {
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
++m_mod_id;
m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
if (notify && m_callback)
@@ -78,9 +78,8 @@
}
}
-void PathMappingList::Insert(ConstString path,
- ConstString replacement, uint32_t index,
- bool notify) {
+void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
+ uint32_t index, bool notify) {
++m_mod_id;
iterator insert_iter;
if (index >= m_pairs.size())
@@ -93,9 +92,8 @@
m_callback(*this, m_callback_baton);
}
-bool PathMappingList::Replace(ConstString path,
- ConstString replacement, uint32_t index,
- bool notify) {
+bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
+ uint32_t index, bool notify) {
if (index >= m_pairs.size())
return false;
++m_mod_id;
@@ -221,20 +219,19 @@
// We must normalize the orig_spec again using the host's path style,
// otherwise there will be mismatch between the host and remote platform
// if they use different path styles.
- if (auto remapped = RemapPath(
- NormalizePath(ConstString(orig_spec.GetCString())).GetStringRef(),
- /*only_if_exists=*/true))
+ if (auto remapped = RemapPath(NormalizePath(orig_spec.GetPath()),
+ /*only_if_exists=*/true))
return remapped;
return {};
}
-bool PathMappingList::Replace(ConstString path,
- ConstString new_path, bool notify) {
+bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
+ bool notify) {
uint32_t idx = FindIndexForPath(path);
if (idx < m_pairs.size()) {
++m_mod_id;
- m_pairs[idx].second = new_path;
+ m_pairs[idx].second = ConstString(new_path);
if (notify && m_callback)
m_callback(*this, m_callback_baton);
return true;
@@ -290,8 +287,8 @@
return false;
}
-uint32_t PathMappingList::FindIndexForPath(ConstString orig_path) const {
- const ConstString path = NormalizePath(orig_path);
+uint32_t PathMappingList::FindIndexForPath(llvm::StringRef orig_path) const {
+ const ConstString path = ConstString(NormalizePath(orig_path));
const_iterator pos;
const_iterator begin = m_pairs.begin();
const_iterator end = m_pairs.end();
Index: lldb/source/Interpreter/OptionValuePathMappings.cpp
===================================================================
--- lldb/source/Interpreter/OptionValuePathMappings.cpp
+++ lldb/source/Interpreter/OptionValuePathMappings.cpp
@@ -62,10 +62,10 @@
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
- ConstString a(orginal_path);
- ConstString b(replace_path);
- if (!m_path_mappings.Replace(a, b, idx, m_notify_changes))
- m_path_mappings.Append(a, b, m_notify_changes);
+ if (!m_path_mappings.Replace(orginal_path, replace_path, idx,
+ m_notify_changes))
+ m_path_mappings.Append(orginal_path, replace_path,
+ m_notify_changes);
changed = true;
} else {
std::string previousError =
@@ -102,9 +102,7 @@
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
- ConstString a(orginal_path);
- ConstString b(replace_path);
- m_path_mappings.Append(a, b, m_notify_changes);
+ m_path_mappings.Append(orginal_path, replace_path, m_notify_changes);
m_value_was_set = true;
changed = true;
} else {
@@ -139,9 +137,8 @@
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
- ConstString a(orginal_path);
- ConstString b(replace_path);
- m_path_mappings.Insert(a, b, idx, m_notify_changes);
+ m_path_mappings.Insert(orginal_path, replace_path, idx,
+ m_notify_changes);
changed = true;
idx++;
} else {
Index: lldb/source/Core/ModuleList.cpp
===================================================================
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -122,8 +122,7 @@
FileSpec resolved;
Status status = FileSystem::Instance().Readlink(symlink, resolved);
if (status.Success())
- m_symlink_paths.Append(ConstString(symlink.GetPath()),
- ConstString(resolved.GetPath()), notify);
+ m_symlink_paths.Append(symlink.GetPath(), resolved.GetPath(), notify);
}
}
Index: lldb/source/Core/Module.cpp
===================================================================
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1614,15 +1614,14 @@
void Module::RegisterXcodeSDK(llvm::StringRef sdk_name, llvm::StringRef sysroot) {
XcodeSDK sdk(sdk_name.str());
- ConstString sdk_path(HostInfo::GetXcodeSDKPath(sdk));
- if (!sdk_path)
+ llvm::StringRef sdk_path(HostInfo::GetXcodeSDKPath(sdk));
+ if (sdk_path.empty())
return;
// If the SDK changed for a previously registered source path, update it.
// This could happend with -fdebug-prefix-map, otherwise it's unlikely.
- ConstString sysroot_cs(sysroot);
- if (!m_source_mappings.Replace(sysroot_cs, sdk_path, true))
+ if (!m_source_mappings.Replace(sysroot, sdk_path, true))
// In the general case, however, append it to the list.
- m_source_mappings.Append(sysroot_cs, sdk_path, false);
+ m_source_mappings.Append(sysroot, sdk_path, false);
}
bool Module::MergeArchitecture(const ArchSpec &arch_spec) {
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1047,8 +1047,7 @@
}
bool last_pair = ((argc - i) == 2);
target->GetImageSearchPathList().Append(
- ConstString(from), ConstString(to),
- last_pair); // Notify if this is the last pair
+ from, to, last_pair); // Notify if this is the last pair
result.SetStatus(eReturnStatusSuccessFinishNoResult);
} else {
if (from[0])
@@ -1175,8 +1174,8 @@
if (from[0] && to[0]) {
bool last_pair = ((argc - i) == 2);
- target->GetImageSearchPathList().Insert(
- ConstString(from), ConstString(to), insert_idx, last_pair);
+ target->GetImageSearchPathList().Insert(from, to, insert_idx,
+ last_pair);
result.SetStatus(eReturnStatusSuccessFinishNoResult);
} else {
if (from[0])
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -214,8 +214,8 @@
if (!target_sp)
return LLDB_RECORD_RESULT(data);
std::string json_str =
- llvm::formatv("{0:2}",
- DebuggerStats::ReportStatistics(target_sp->GetDebugger(),
+ llvm::formatv("{0:2}",
+ DebuggerStats::ReportStatistics(target_sp->GetDebugger(),
target_sp.get())).str();
data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str));
return LLDB_RECORD_RESULT(data);
@@ -1586,13 +1586,13 @@
if (!target_sp)
return error.SetErrorString("invalid target");
- const ConstString csFrom(from), csTo(to);
- if (!csFrom)
+ llvm::StringRef srFrom = from, srTo = to;
+ if (srFrom.empty())
return error.SetErrorString("<from> path can't be empty");
- if (!csTo)
+ if (srTo.empty())
return error.SetErrorString("<to> path can't be empty");
- target_sp->GetImageSearchPathList().Append(csFrom, csTo, true);
+ target_sp->GetImageSearchPathList().Append(srFrom, srTo, true);
}
lldb::SBModule SBTarget::AddModule(const char *path, const char *triple,
Index: lldb/include/lldb/Target/PathMappingList.h
===================================================================
--- lldb/include/lldb/Target/PathMappingList.h
+++ lldb/include/lldb/Target/PathMappingList.h
@@ -32,8 +32,7 @@
const PathMappingList &operator=(const PathMappingList &rhs);
- void Append(ConstString path, ConstString replacement,
- bool notify);
+ void Append(llvm::StringRef path, llvm::StringRef replacement, bool notify);
void Append(const PathMappingList &rhs, bool notify);
@@ -49,17 +48,16 @@
bool GetPathsAtIndex(uint32_t idx, ConstString &path,
ConstString &new_path) const;
- void Insert(ConstString path, ConstString replacement,
+ void Insert(llvm::StringRef path, llvm::StringRef replacement,
uint32_t insert_idx, bool notify);
bool Remove(size_t index, bool notify);
bool Remove(ConstString path, bool notify);
- bool Replace(ConstString path, ConstString replacement,
- bool notify);
+ bool Replace(llvm::StringRef path, llvm::StringRef replacement, bool notify);
- bool Replace(ConstString path, ConstString replacement,
+ bool Replace(llvm::StringRef path, llvm::StringRef replacement,
uint32_t index, bool notify);
bool RemapPath(ConstString path, ConstString &new_path) const;
@@ -104,7 +102,7 @@
/// The newly remapped filespec that is guaranteed to exist.
llvm::Optional<FileSpec> FindFile(const FileSpec &orig_spec) const;
- uint32_t FindIndexForPath(ConstString path) const;
+ uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const { return m_mod_id; }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
