[Lldb-commits] [lldb] a346372 - Change PathMappingList::FindFile to return an optional result (NFC)

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2021-06-29T15:10:46-07:00
New Revision: a346372200e7b2b99631bd90691678d5ca03fdd1

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

LOG: Change PathMappingList::FindFile to return an optional result (NFC)

This is an NFC modernization refactoring that replaces the combination
of a bool return + reference argument, with an Optional return value.

Differential Revision: https://reviews.llvm.org/D104405

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h
index 5d8e2a1b4d242..46d7a427d3071 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -90,14 +90,9 @@ class PathMappingList {
   /// \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 &orig_spec, FileSpec &new_spec) const;
+  ///     The newly remapped filespec that is guaranteed to exist.
+  llvm::Optional<FileSpec> FindFile(const FileSpec &orig_spec) const;
 
   uint32_t FindIndexForPath(ConstString path) const;
 

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 6502518f9247f..af7128496812d 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1598,7 +1598,11 @@ bool Module::MatchesModuleSpec(const ModuleSpec &module_ref) {
 bool Module::FindSourceFile(const FileSpec &orig_spec,
                             FileSpec &new_spec) const {
   std::lock_guard<std::recursive_mutex> 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,

diff  --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index 691bda6592e2e..3d51d42897c23 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -441,13 +441,17 @@ void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
       }
       // 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);
         }
       }

diff  --git a/lldb/source/Symbol/LineEntry.cpp b/lldb/source/Symbol/LineEntry.cpp
index 58bf8509a9589..1b2801cd03683 100644
--- a/lldb/source/Symbol/LineEntry.cpp
+++ b/lldb/source/Symbol/LineEntry.cpp
@@ -252,9 +252,9 @@ AddressRange LineEntry::GetSameLineContiguousAddressRange(
 
 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;
   }
 }

diff  --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp
index b6dbf551ea57d..f9d415bcf15d7 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -194,16 +194,16 @@ bool PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) co
   return false;
 }
 
-bool PathMappingList::FindFile(const FileSpec &orig_spec,
-                               FileSpec &new_spec) const {
+llvm::Optional<FileSpec>
+PathMappingList::FindFile(const FileSpec &orig_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 @@ bool PathMappingList::FindFile(const FileSpec &orig_spec,
       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,