Address most of Jim's comments

Author: royitaqi

lldb/include/lldb/Symbol/CompileUnit.h

@@ -19,12 +19,12 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 
 namespace lldb_private {
-class RealpathPrefixes;
 
 /// \class CompileUnit CompileUnit.h "lldb/Symbol/CompileUnit.h"
 /// A class that describes a compilation unit.

lldb/include/lldb/Target/Statistics.h

@@ -10,6 +10,7 @@
 #define LLDB_TARGET_STATISTICS_H
 
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RealpathPrefixes.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringMap.h"
@@ -175,7 +176,7 @@ struct StatisticsOptions {
 };
 
 /// A class that represents statistics for a since lldb_private::Target.
-class TargetStats {
+class TargetStats : public RealpathPrefixesStats {
 public:
   llvm::json::Value ToJSON(Target &target,
                            const lldb_private::StatisticsOptions &options);
@@ -184,8 +185,8 @@ class TargetStats {
   void SetFirstPrivateStopTime();
   void SetFirstPublicStopTime();
   void IncreaseSourceMapDeduceCount();
-  void IncreaseSourceRealpathAttemptCount();
-  void IncreaseSourceRealpathCompatibleCount();
+  virtual void IncreaseSourceRealpathAttemptCount() override;
+  virtual void IncreaseSourceRealpathCompatibleCount() override;
 
   StatsDuration &GetCreateTime() { return m_create_time; }
   StatsSuccessFail &GetExpressionStats() { return m_expr_eval; }

lldb/include/lldb/Utility/FileSpecList.h

@@ -17,7 +17,6 @@
 #include <vector>
 
 namespace lldb_private {
-class RealpathPrefixes;
 class Stream;
 
 /// A list of support files for a CompileUnit.

lldb/include/lldb/Utility/RealpathPrefixes.h

@@ -9,19 +9,14 @@
 #ifndef LLDB_CORE_REALPATHPREFIXES_H
 #define LLDB_CORE_REALPATHPREFIXES_H
 
+#include "lldb/lldb-forward.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include <optional>
 #include <string>
 #include <vector>
 
-namespace lldb_private {
-class FileSpec;
-class FileSpecList;
-class Target;
-} // namespace lldb_private
-
 namespace lldb_private {
 
 class RealpathPrefixes {
@@ -37,8 +32,8 @@ class RealpathPrefixes {
   void SetFileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
 
   // Sets an optional Target instance to gather statistics.
-  void SetTarget(Target *target) { m_target = target; }
-  Target *GetTarget() const { return m_target; }
+  void SetTarget(const lldb::TargetSP& target) { m_target = target; }
+  lldb::TargetSP GetTarget() const { return m_target.lock(); }
 
   std::optional<FileSpec> ResolveSymlinks(const FileSpec &file_spec) const;
 
@@ -55,7 +50,15 @@ class RealpathPrefixes {
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
 
   // The optional Target instance to gather statistics.
-  Target *m_target;
+  lldb::TargetWP m_target;
+};
+
+class RealpathPrefixesStats {
+public:
+  virtual ~RealpathPrefixesStats() = default;
+
+  virtual void IncreaseSourceRealpathAttemptCount() = 0;
+  virtual void IncreaseSourceRealpathCompatibleCount() = 0;
 };
 
 } // namespace lldb_private

lldb/include/lldb/lldb-forward.h

@@ -175,6 +175,7 @@ class Queue;
 class QueueImpl;
 class QueueItem;
 class REPL;
+class RealpathPrefixes;
 class RecognizedStackFrame;
 class RegisterCheckpoint;
 class RegisterContext;

lldb/source/Target/Target.cpp

@@ -4360,7 +4360,7 @@ InlineStrategy TargetProperties::GetInlineStrategy() const {
 RealpathPrefixes TargetProperties::GetSourceRealpathPrefixes() const {
   const uint32_t idx = ePropertySourceRealpathPrefixes;
   auto prefixes = RealpathPrefixes(GetPropertyAtIndexAs<FileSpecList>(idx, {}));
-  prefixes.SetTarget(this->m_target);
+  prefixes.SetTarget(this->m_target->shared_from_this());
   return prefixes;
 }
 

lldb/source/Target/TargetProperties.td

@@ -152,7 +152,7 @@ let Definition = "target" in {
     Desc<"The strategy to use when settings breakpoints by file and line. Breakpoint locations can end up being inlined by the compiler, so that a compile unit 'a.c' might contain an inlined function from another source file. Usually this is limited to breakpoint locations from inlined functions from header or other include files, or more accurately non-implementation source files. Sometimes code might #include implementation files and cause inlined breakpoint locations in inlined implementation files. Always checking for inlined breakpoint locations can be expensive (memory and time), so if you have a project with many headers and find that setting breakpoints is slow, then you can change this setting to headers. This setting allows you to control exactly which strategy is used when setting file and line breakpoints.">;
   def SourceRealpathPrefixes: Property<"source-realpath-prefixes", "FileSpecList">,
     DefaultStringValue<"">,
-    Desc<"Realpath any source paths that start with one of these prefixes. If the debug info contains symlinks that don't match the original source file locations that the user will use to set breakpoints, then this setting can help resolve breakpoints correctly.">;
+    Desc<"Realpath any source paths that start with one of these prefixes. If the debug info contains symlinks which match the original source file's basename but don't match its location that the user will use to set breakpoints, then this setting can help resolve breakpoints correctly. This handles both symlinked files and directories. Wild card prefixes: An empty string matches all paths. A forward slash matches absolute paths.">;
   def DisassemblyFlavor: Property<"x86-disassembly-flavor", "Enum">,
     DefaultEnumValue<"eX86DisFlavorDefault">,
     EnumValues<"OptionEnumValues(g_x86_dis_flavor_value_types)">,

lldb/source/Utility/FileSpecList.cpp

@@ -116,7 +116,7 @@ size_t SupportFileList::FindFileIndex(size_t start_idx,
 enum IsCompatibleResult {
   kNoMatch = 0,
   kOnlyFileMatch = 1,
-  kCompatible = 2,
+  kBothDirectoryAndFileMatch = 2,
 };
 
 IsCompatibleResult IsCompatible(const FileSpec &curr_file,
@@ -132,15 +132,15 @@ IsCompatibleResult IsCompatible(const FileSpec &curr_file,
     return IsCompatibleResult::kNoMatch;
 
   // Only compare the full name if the we were asked to and if the current
-  // file entry has the a directory. If it doesn't have a directory then we
-  // only compare the filename.
+  // file entry has a directory. If it doesn't have a directory then we only
+  // compare the filename.
   if (FileSpec::Equal(curr_file, file_spec, full)) {
-    return IsCompatibleResult::kCompatible;
+    return IsCompatibleResult::kBothDirectoryAndFileMatch;
   } else if (curr_file.IsRelative() || file_spec_relative) {
     llvm::StringRef curr_file_dir = curr_file.GetDirectory().GetStringRef();
     if (curr_file_dir.empty())
-      return IsCompatibleResult::kCompatible; // Basename match only for this
-                                              // file in the list
+      // Basename match only for this file in the list
+      return IsCompatibleResult::kBothDirectoryAndFileMatch;
 
     // Check if we have a relative path in our file list, or if "file_spec" is
     // relative, if so, check if either ends with the other.
@@ -158,7 +158,7 @@ IsCompatibleResult IsCompatible(const FileSpec &curr_file,
         file_spec_case_sensitive || curr_file.IsCaseSensitive();
     if (is_suffix(curr_file_dir, file_spec_dir, case_sensitive) ||
         is_suffix(file_spec_dir, curr_file_dir, case_sensitive))
-      return IsCompatibleResult::kCompatible;
+      return IsCompatibleResult::kBothDirectoryAndFileMatch;
   }
   return IsCompatibleResult::kOnlyFileMatch;
 }
@@ -174,15 +174,15 @@ size_t SupportFileList::FindCompatibleIndex(
     const FileSpec &curr_file = m_files[idx]->GetSpecOnly();
 
     IsCompatibleResult result = IsCompatible(curr_file, file_spec);
-    if (result == IsCompatibleResult::kCompatible)
+    if (result == IsCompatibleResult::kBothDirectoryAndFileMatch)
       return idx;
 
     if (realpath_prefixes && result == IsCompatibleResult::kOnlyFileMatch) {
       if (std::optional<FileSpec> resolved_curr_file =
               realpath_prefixes->ResolveSymlinks(curr_file)) {
         if (IsCompatible(*resolved_curr_file, file_spec)) {
           // Stats and logging.
-          if (Target *target = realpath_prefixes->GetTarget())
+          if (lldb::TargetSP target = realpath_prefixes->GetTarget())
             target->GetStatistics().IncreaseSourceRealpathCompatibleCount();
           Log *log = GetLog(LLDBLog::Source);
           LLDB_LOGF(log,

lldb/source/Utility/RealpathPrefixes.cpp

@@ -18,7 +18,7 @@
 using namespace lldb_private;
 
 RealpathPrefixes::RealpathPrefixes(const FileSpecList &file_spec_list)
-    : m_fs(llvm::vfs::getRealFileSystem()), m_target(nullptr) {
+    : m_fs(llvm::vfs::getRealFileSystem()) {
   m_prefixes.reserve(file_spec_list.GetSize());
   for (const FileSpec &file_spec : file_spec_list) {
     m_prefixes.emplace_back(file_spec.GetPath());
@@ -43,8 +43,8 @@ RealpathPrefixes::ResolveSymlinks(const FileSpec &file_spec) const {
   for (const std::string &prefix : m_prefixes) {
     if (is_prefix(file_spec_path, prefix, file_spec.IsCaseSensitive())) {
       // Stats and logging.
-      if (m_target)
-        m_target->GetStatistics().IncreaseSourceRealpathAttemptCount();
+      if (lldb::TargetSP target = m_target.lock())
+        target->GetStatistics().IncreaseSourceRealpathAttemptCount();
       Log *log = GetLog(LLDBLog::Source);
       LLDB_LOGF(log, "Realpath'ing support file %s", file_spec_path.c_str());