[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa346372200e7: Change PathMappingList::FindFile to return an
optional result (NFC) (authored by aprantl).
Herald added a project: LLDB.
Changed prior to commit:
https://reviews.llvm.org/D104405?vs=353700=355379#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104405/new/
https://reviews.llvm.org/D104405
Files:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/Core/Module.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Symbol/LineEntry.cpp
lldb/source/Target/PathMappingList.cpp
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -194,16 +194,16 @@
return false;
}
-bool PathMappingList::FindFile(const FileSpec _spec,
- FileSpec _spec) const {
+llvm::Optional
+PathMappingList::FindFile(const FileSpec _spec) const {
if (m_pairs.empty())
-return false;
-
+return {};
+
std::string orig_path = orig_spec.GetPath();
if (orig_path.empty())
-return false;
-
+return {};
+
bool orig_is_relative = orig_spec.IsRelative();
for (auto entry : m_pairs) {
@@ -228,15 +228,15 @@
continue;
if (orig_ref.consume_front(prefix_ref)) {
+ FileSpec new_spec;
new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
new_spec.AppendPathComponent(orig_ref);
if (FileSystem::Instance().Exists(new_spec))
-return true;
+return new_spec;
}
}
- new_spec.Clear();
- return false;
+ return {};
}
bool PathMappingList::Replace(ConstString path,
Index: lldb/source/Symbol/LineEntry.cpp
===
--- lldb/source/Symbol/LineEntry.cpp
+++ lldb/source/Symbol/LineEntry.cpp
@@ -252,9 +252,9 @@
void LineEntry::ApplyFileMappings(lldb::TargetSP target_sp) {
if (target_sp) {
-// Apply any file remappings to our file
-FileSpec new_file_spec;
-if (target_sp->GetSourcePathMap().FindFile(original_file, new_file_spec))
- file = new_file_spec;
+// Apply any file remappings to our file.
+if (auto new_file_spec =
+target_sp->GetSourcePathMap().FindFile(original_file))
+ file = *new_file_spec;
}
}
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -441,13 +441,17 @@
}
// Try remapping if m_file_spec does not correspond to an existing file.
if (!FileSystem::Instance().Exists(m_file_spec)) {
-FileSpec new_file_spec;
-// Check target specific source remappings first, then fall back to
-// modules objects can have individual path remappings that were
-// detected when the debug info for a module was found. then
-if (target->GetSourcePathMap().FindFile(m_file_spec, new_file_spec) ||
-target->GetImages().FindSourceFile(m_file_spec, new_file_spec)) {
- m_file_spec = new_file_spec;
+// Check target specific source remappings (i.e., the
+// target.source-map setting), then fall back to the module
+// specific remapping (i.e., the .dSYM remapping dictionary).
+auto remapped = target->GetSourcePathMap().FindFile(m_file_spec);
+if (!remapped) {
+ FileSpec new_spec;
+ if (target->GetImages().FindSourceFile(m_file_spec, new_spec))
+remapped = new_spec;
+}
+if (remapped) {
+ m_file_spec = *remapped;
m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
}
}
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1598,7 +1598,11 @@
bool Module::FindSourceFile(const FileSpec _spec,
FileSpec _spec) const {
std::lock_guard guard(m_mutex);
- return m_source_mappings.FindFile(orig_spec, new_spec);
+ if (auto remapped = m_source_mappings.FindFile(orig_spec)) {
+new_spec = *remapped;
+return true;
+ }
+ return false;
}
bool Module::RemapSourceFile(llvm::StringRef path,
Index: lldb/include/lldb/Target/PathMappingList.h
===
--- lldb/include/lldb/Target/PathMappingList.h
+++ lldb/include/lldb/Target/PathMappingList.h
@@ -90,14 +90,9 @@
/// \param[in] orig_spec
/// The original source file path to try and remap.
///
- /// \param[out] new_spec
- /// The newly remapped filespec that is guaranteed to exist.
- ///
/// \return
- /// /b true if \a orig_spec
[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)
JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM modulo formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104405/new/ https://reviews.llvm.org/D104405 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: lldb/source/Core/SourceManager.cpp:444
if (!FileSystem::Instance().Exists(m_file_spec)) {
-FileSpec new_file_spec;
// Check target specific source remappings first, then fall back to
// modules objects can have individual path remappings that were
This whole comment reads awkward and does not totally make sense and ends with
`. then`
Comment at: lldb/source/Target/PathMappingList.cpp:231
if (orig_ref.consume_front(prefix_ref)) {
new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
new_spec.AppendPathComponent(orig_ref);
Make sense to move `FileSpec new_spec;` declaration down to here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104405/new/
https://reviews.llvm.org/D104405
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)
aprantl added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104405/new/ https://reviews.llvm.org/D104405 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)
aprantl updated this revision to Diff 353700.
aprantl added a comment.
Update to the correct patch
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104405/new/
https://reviews.llvm.org/D104405
Files:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/Core/Module.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Symbol/LineEntry.cpp
lldb/source/Target/PathMappingList.cpp
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -193,15 +193,16 @@
return false;
}
-bool PathMappingList::FindFile(const FileSpec _spec,
- FileSpec _spec) const {
+llvm::Optional
+PathMappingList::FindFile(const FileSpec _spec) const {
+ FileSpec new_spec;
if (m_pairs.empty())
-return false;
+return {};
std::string orig_path = orig_spec.GetPath();
if (orig_path.empty())
-return false;
+return {};
bool orig_is_relative = orig_spec.IsRelative();
@@ -230,12 +231,12 @@
new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
new_spec.AppendPathComponent(orig_ref);
if (FileSystem::Instance().Exists(new_spec))
-return true;
+return new_spec;
}
}
new_spec.Clear();
- return false;
+ return {};
}
bool PathMappingList::Replace(ConstString path,
Index: lldb/source/Symbol/LineEntry.cpp
===
--- lldb/source/Symbol/LineEntry.cpp
+++ lldb/source/Symbol/LineEntry.cpp
@@ -252,9 +252,8 @@
void LineEntry::ApplyFileMappings(lldb::TargetSP target_sp) {
if (target_sp) {
-// Apply any file remappings to our file
-FileSpec new_file_spec;
-if (target_sp->GetSourcePathMap().FindFile(original_file, new_file_spec))
- file = new_file_spec;
+// Apply any file remappings to our file.
+if (auto new_file_spec = target_sp->GetSourcePathMap().FindFile(original_file))
+ file = *new_file_spec;
}
}
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -441,13 +441,17 @@
}
// Try remapping if m_file_spec does not correspond to an existing file.
if (!FileSystem::Instance().Exists(m_file_spec)) {
-FileSpec new_file_spec;
// Check target specific source remappings first, then fall back to
// modules objects can have individual path remappings that were
// detected when the debug info for a module was found. then
-if (target->GetSourcePathMap().FindFile(m_file_spec, new_file_spec) ||
-target->GetImages().FindSourceFile(m_file_spec, new_file_spec)) {
- m_file_spec = new_file_spec;
+auto remapped = target->GetSourcePathMap().FindFile(m_file_spec);
+if (!remapped) {
+ FileSpec new_spec;
+ if (target->GetImages().FindSourceFile(m_file_spec, new_spec))
+remapped = new_spec;
+}
+if (remapped) {
+ m_file_spec = *remapped;
m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
}
}
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1598,7 +1598,11 @@
bool Module::FindSourceFile(const FileSpec _spec,
FileSpec _spec) const {
std::lock_guard guard(m_mutex);
- return m_source_mappings.FindFile(orig_spec, new_spec);
+ if (auto remapped = m_source_mappings.FindFile(orig_spec)) {
+new_spec = *remapped;
+return true;
+ }
+ return false;
}
bool Module::RemapSourceFile(llvm::StringRef path,
Index: lldb/include/lldb/Target/PathMappingList.h
===
--- lldb/include/lldb/Target/PathMappingList.h
+++ lldb/include/lldb/Target/PathMappingList.h
@@ -90,14 +90,9 @@
/// \param[in] orig_spec
/// The original source file path to try and remap.
///
- /// \param[out] new_spec
- /// The newly remapped filespec that is guaranteed to exist.
- ///
/// \return
- /// /b true if \a orig_spec was successfully located and
- /// \a new_spec is filled in with an existing file spec,
- /// \b false otherwise.
- bool FindFile(const FileSpec _spec, FileSpec _spec) const;
+ /// The newly remapped filespec that is guaranteed to exist.
+ llvm::Optional FindFile(const FileSpec _spec) const;
uint32_t FindIndexForPath(ConstString path) const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)
aprantl created this revision.
aprantl added reviewers: JDevlieghere, shafik, jingham.
aprantl requested review of this revision.
This is an NFC modernization refactoring that replaces the combination of a
bool return + reference argument, with an Optional return value.
https://reviews.llvm.org/D104405
Files:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/Core/Module.cpp
lldb/source/Target/PathMappingList.cpp
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -145,18 +145,16 @@
bool PathMappingList::RemapPath(ConstString path,
ConstString _path) const {
- std::string remapped;
- if (RemapPath(path.GetStringRef(), remapped)) {
-new_path.SetString(remapped);
+ if (auto remapped = RemapPath(path.GetStringRef())) {
+new_path.SetString(remapped->GetPath());
return true;
}
return false;
}
-bool PathMappingList::RemapPath(llvm::StringRef path,
-std::string _path) const {
+llvm::Optional PathMappingList::RemapPath(llvm::StringRef path)
const{
if (m_pairs.empty() || path.empty())
-return false;
+return {};
LazyBool path_is_relative = eLazyBoolCalculate;
for (const auto : m_pairs) {
auto prefix = it.first.GetStringRef();
@@ -177,10 +175,9 @@
}
FileSpec remapped(it.second.GetStringRef());
remapped.AppendPathComponent(path);
-new_path = remapped.GetPath();
-return true;
+return remapped;
}
- return false;
+ return {};
}
bool PathMappingList::ReverseRemapPath(const FileSpec , FileSpec )
const {
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1604,7 +1604,11 @@
bool Module::RemapSourceFile(llvm::StringRef path,
std::string _path) const {
std::lock_guard guard(m_mutex);
- return m_source_mappings.RemapPath(path, new_path);
+ if (auto remapped = m_source_mappings.RemapPath(path)) {
+new_path = remapped->GetPath();
+return true;
+ }
+ return false;
}
void Module::RegisterXcodeSDK(llvm::StringRef sdk_name, llvm::StringRef
sysroot) {
Index: lldb/include/lldb/Target/PathMappingList.h
===
--- lldb/include/lldb/Target/PathMappingList.h
+++ lldb/include/lldb/Target/PathMappingList.h
@@ -72,13 +72,9 @@
/// \param[in] path
/// The original source file path to try and remap.
///
- /// \param[out] new_path
- /// The newly remapped filespec that is may or may not exist.
- ///
/// \return
- /// /b true if \a path was successfully located and \a new_path
- /// is filled in with a new source path, \b false otherwise.
- bool RemapPath(llvm::StringRef path, std::string _path) const;
+ /// The remapped filespec that may or may not exist on disk.
+ llvm::Optional RemapPath(llvm::StringRef path) const;
bool RemapPath(const char *, std::string &) const = delete;
bool ReverseRemapPath(const FileSpec , FileSpec ) const;
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -145,18 +145,16 @@
bool PathMappingList::RemapPath(ConstString path,
ConstString _path) const {
- std::string remapped;
- if (RemapPath(path.GetStringRef(), remapped)) {
-new_path.SetString(remapped);
+ if (auto remapped = RemapPath(path.GetStringRef())) {
+new_path.SetString(remapped->GetPath());
return true;
}
return false;
}
-bool PathMappingList::RemapPath(llvm::StringRef path,
-std::string _path) const {
+llvm::Optional PathMappingList::RemapPath(llvm::StringRef path) const{
if (m_pairs.empty() || path.empty())
-return false;
+return {};
LazyBool path_is_relative = eLazyBoolCalculate;
for (const auto : m_pairs) {
auto prefix = it.first.GetStringRef();
@@ -177,10 +175,9 @@
}
FileSpec remapped(it.second.GetStringRef());
remapped.AppendPathComponent(path);
-new_path = remapped.GetPath();
-return true;
+return remapped;
}
- return false;
+ return {};
}
bool PathMappingList::ReverseRemapPath(const FileSpec , FileSpec ) const {
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1604,7 +1604,11 @@
bool Module::RemapSourceFile(llvm::StringRef path,
std::string _path) const {
std::lock_guard guard(m_mutex);
- return m_source_mappings.RemapPath(path, new_path);
+ if (auto remapped = m_source_mappings.RemapPath(path)) {
+
