Improve path remapping in cross-debugging scenarios

[aprantl]

Summary

This patch implements a slight improvement when debugging across platforms and remapping source paths that are in a non-native format. See the unit test for examples.

rdar://79205675

[aprantl]

Ping.

[JDevlieghere]

LGTM with a few nits

lldb/source/Target/PathMappingList.cpp

160

Can we rely on the path being normalized? If not, should this be while instead of if?

204–208

orig_file?

lldb/source/Target/PathMappingList.cpp

bool PathMappingList::RemapPath(ConstString path,
                                 ConstString &new_path) const {
   if (llvm::Optional<FileSpec> remapped = RemapPath(path.GetStringRef())) {
     new_path.SetString(remapped->GetPath());
     return true;
   }
   return false;
 }
 
+/// Append components to path, applying style.
+static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
+                                 llvm::sys::path::Style style) {
+    auto component = llvm::sys::path::begin(components, style);
+    auto e = llvm::sys::path::end(components);
+    while (component != e &&
+        llvm::sys::path::is_separator(*component->data(), style))
+      ++component;
+    for (; component != e; ++component)
+      path.AppendPathComponent(*component);
+}
+
 llvm::Optional<FileSpec>
 PathMappingList::RemapPath(llvm::StringRef path) const {
   if (m_pairs.empty() || path.empty())
     return {};
   LazyBool path_is_relative = eLazyBoolCalculate;
   for (const auto &it : m_pairs) {
     auto prefix = it.first.GetStringRef();
     if (!path.consume_front(prefix)) {
       // Relative paths won't have a leading "./" in them unless "." is the
       // only thing in the relative path so we need to work around "."
       // carefully.
       if (prefix != ".")
         continue;
       // We need to figure out if the "path" argument is relative. If it is,
       // then we should remap, else skip this entry.
       if (path_is_relative == eLazyBoolCalculate) {
         path_is_relative =
             FileSpec(path).IsRelative() ? eLazyBoolYes : eLazyBoolNo;
       }
       if (!path_is_relative)
         continue;
     }
     FileSpec remapped(it.second.GetStringRef());
-    remapped.AppendPathComponent(path);
+    auto orig_style = FileSpec::GuessPathStyle(prefix).getValueOr(
+        llvm::sys::path::Style::native);
+    AppendPathComponents(remapped, path, orig_style);
     return remapped;
   }
   return {};
 }
 
 bool PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
   std::string path = file.GetPath();
   llvm::StringRef path_ref(path);
   for (const auto &it : m_pairs) {
     if (!path_ref.consume_front(it.second.GetStringRef()))
       continue;
-    fixed.SetFile(it.first.GetStringRef(), FileSpec::Style::native);
-    fixed.AppendPathComponent(path_ref);
+    auto orig_file = it.first.GetStringRef();
+    auto orig_style = FileSpec::GuessPathStyle(orig_file).getValueOr(
+        llvm::sys::path::Style::native);
+    fixed.SetFile(orig_file, orig_style);
+    AppendPathComponents(fixed, path_ref, orig_style);
     return true;
   }
   return false;
 }
 
 
 llvm::Optional<FileSpec> PathMappingList::FindFile(const FileSpec &orig_spec) const {
   if (auto remapped = RemapPath(orig_spec.GetPath()))

lldb/unittests/Target/PathMappingListTest.cpp

 //===-- PathMappingListTest.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 "llvm/ADT/ArrayRef.h"
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "gtest/gtest.h"
 #include <utility>
 
 using namespace lldb_private;
 
 namespace {
 struct Matches {
   FileSpec original;
   FileSpec remapped;
   Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+      : original(o, style), remapped(r) {}
 };
 } // namespace
 
 static void TestPathMappings(const PathMappingList &map,
                              llvm::ArrayRef<Matches> matches,
                              llvm::ArrayRef<ConstString> fails) {
   ConstString actual_remapped;
   for (const auto &fail : fails) {
TEST(PathMappingListTest, RemapRoot) {
   ConstString fails[] = {
     ConstString("foo.c"),
     ConstString("./foo.c"),
     ConstString("../foo.c"),
     ConstString("../bar/foo.c"),
   };
   TestPathMappings(map, matches, fails);
 }
+
+#ifndef _WIN32
+TEST(PathMappingListTest, CrossPlatformTests) {
+  PathMappingList map;
+  map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+  Matches matches[] = {
+    {R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
+    {R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
+    {R"(C:\old\foo\.)", llvm::sys::path::Style::windows, "/new/foo"},
+    {R"(C:\old\foo.c)", llvm::sys::path::Style::windows, "/new/foo.c"},
+    {R"(C:\old\foo.c\.)", llvm::sys::path::Style::windows, "/new/foo.c"},
+    {R"(C:\old\.\foo.c)", llvm::sys::path::Style::windows, "/new/foo.c"},
+  };
+  ConstString fails[] = {
+    ConstString("/foo"),
+    ConstString("/"),
+    ConstString("foo.c"),
+    ConstString("./foo.c"),
+    ConstString("../foo.c"),
+    ConstString("../bar/foo.c"),
+  };
+  TestPathMappings(map, matches, fails);
+}
+#endif