[lldb] Move MD5 Checksum from FileSpec to SupportFile

When I added the MD5 checksum I was on the fence between storing it in
FileSpec or creating a new SupportFile abstraction. The latter was
deemed overkill for just the MD5 hashes, but support for inline sources
in the DWARF 5 line table tipped the scales. This patch moves the MD5
checksum into the new SupportFile class.

Author: JDevlieghere

lldb/include/lldb/Utility/FileSpec.h

@@ -13,7 +13,6 @@
 #include <optional>
 #include <string>
 
-#include "lldb/Utility/Checksum.h"
 #include "lldb/Utility/ConstString.h"
 
 #include "llvm/ADT/StringRef.h"
@@ -72,12 +71,8 @@ class FileSpec {
   /// \param[in] style
   ///     The style of the path
   ///
-  /// \param[in] checksum
-  ///     The MD5 checksum of the path.
-  ///
   /// \see FileSpec::SetFile (const char *path)
-  explicit FileSpec(llvm::StringRef path, Style style = Style::native,
-                    const Checksum &checksum = {});
+  explicit FileSpec(llvm::StringRef path, Style style = Style::native);
 
   explicit FileSpec(llvm::StringRef path, const llvm::Triple &triple);
 
@@ -367,11 +362,7 @@ class FileSpec {
   ///
   /// \param[in] style
   ///     The style for the given path.
-  ///
-  /// \param[in] checksum
-  ///     The checksum for the given path.
-  void SetFile(llvm::StringRef path, Style style,
-               const Checksum &checksum = {});
+  void SetFile(llvm::StringRef path, Style style);
 
   /// Change the file specified with a new path.
   ///
@@ -414,19 +405,13 @@ class FileSpec {
   ///   The lifetime of the StringRefs is tied to the lifetime of the FileSpec.
   std::vector<llvm::StringRef> GetComponents() const;
 
-  /// Return the checksum for this FileSpec or all zeros if there is none.
-  const Checksum &GetChecksum() const { return m_checksum; };
-
 protected:
   // Convenience method for setting the file without changing the style.
   void SetFile(llvm::StringRef path);
 
   /// Called anytime m_directory or m_filename is changed to clear any cached
   /// state in this object.
-  void PathWasModified() {
-    m_checksum = Checksum();
-    m_absolute = Absolute::Calculate;
-  }
+  void PathWasModified() { m_absolute = Absolute::Calculate; }
 
   enum class Absolute : uint8_t {
     Calculate,
@@ -440,9 +425,6 @@ class FileSpec {
   /// The unique'd filename path.
   ConstString m_filename;
 
-  /// The optional MD5 checksum of the file.
-  Checksum m_checksum;
-
   /// Cache whether this path is absolute.
   mutable Absolute m_absolute = Absolute::Calculate;
 

lldb/include/lldb/Utility/FileSpecList.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_CORE_FILESPECLIST_H
 #define LLDB_CORE_FILESPECLIST_H
 
+#include "lldb/Utility/Checksum.h"
 #include "lldb/Utility/FileSpec.h"
 
 #include <cstddef>
@@ -24,17 +25,22 @@ class Stream;
 class SupportFile {
 protected:
   FileSpec m_file_spec;
+  Checksum m_checksum;
 
 public:
-  SupportFile(const FileSpec &spec) : m_file_spec(spec) {}
+  SupportFile(const FileSpec &spec) : m_file_spec(spec), m_checksum() {}
+  SupportFile(const FileSpec &spec, const Checksum &checksum)
+      : m_file_spec(spec), m_checksum(checksum) {}
   SupportFile(const SupportFile &other) = delete;
   SupportFile(SupportFile &&other) = default;
   virtual ~SupportFile() = default;
   bool operator==(const SupportFile &other) {
-    return m_file_spec == other.m_file_spec;
+    return m_file_spec == other.m_file_spec && m_checksum == other.m_checksum;
   }
   /// Return the file name only. Useful for resolving breakpoints by file name.
   const FileSpec &GetSpecOnly() const { return m_file_spec; };
+  /// Return the checksum or all zeros if there is none.
+  const Checksum &GetChecksum() const { return m_checksum; };
   /// Materialize the file to disk and return the path to that temporary file.
   virtual const FileSpec &Materialize() { return m_file_spec; }
 };
@@ -90,7 +96,7 @@ class SupportFileList {
 
   template <class... Args> void EmplaceBack(Args &&...args) {
     m_files.push_back(
-        std::make_unique<SupportFile>(FileSpec(std::forward<Args>(args)...)));
+        std::make_unique<SupportFile>(std::forward<Args>(args)...));
   }
 
 protected:

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -294,7 +294,7 @@ static void ParseSupportFilesFromPrologue(
     }
 
     // Unconditionally add an entry, so the indices match up.
-    support_files.EmplaceBack(remapped_file, style, checksum);
+    support_files.EmplaceBack(FileSpec(remapped_file, style), checksum);
   }
 }
 

lldb/source/Utility/FileSpec.cpp

@@ -68,9 +68,8 @@ void Denormalize(llvm::SmallVectorImpl<char> &path, FileSpec::Style style) {
 FileSpec::FileSpec() : m_style(GetNativeStyle()) {}
 
 // Default constructor that can take an optional full path to a file on disk.
-FileSpec::FileSpec(llvm::StringRef path, Style style, const Checksum &checksum)
-    : m_checksum(checksum), m_style(style) {
-  SetFile(path, style, checksum);
+FileSpec::FileSpec(llvm::StringRef path, Style style) : m_style(style) {
+  SetFile(path, style);
 }
 
 FileSpec::FileSpec(llvm::StringRef path, const llvm::Triple &triple)
@@ -172,11 +171,9 @@ void FileSpec::SetFile(llvm::StringRef pathname) { SetFile(pathname, m_style); }
 // Update the contents of this object with a new path. The path will be split
 // up into a directory and filename and stored as uniqued string values for
 // quick comparison and efficient memory usage.
-void FileSpec::SetFile(llvm::StringRef pathname, Style style,
-                       const Checksum &checksum) {
+void FileSpec::SetFile(llvm::StringRef pathname, Style style) {
   Clear();
   m_style = (style == Style::native) ? GetNativeStyle() : style;
-  m_checksum = checksum;
 
   if (pathname.empty())
     return;

lldb/unittests/Utility/FileSpecTest.cpp

@@ -534,16 +534,3 @@ TEST(FileSpecTest, TestGetComponents) {
     EXPECT_EQ(file_spec.GetComponents(), pair.second);
   }
 }
-
-TEST(FileSpecTest, TestChecksum) {
-  Checksum checksum(llvm::MD5::MD5Result{
-      {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}});
-  FileSpec file_spec("/foo/bar", FileSpec::Style::posix, checksum);
-  EXPECT_TRUE(static_cast<bool>(file_spec.GetChecksum()));
-  EXPECT_EQ(file_spec.GetChecksum(), checksum);
-
-  FileSpec copy = file_spec;
-
-  EXPECT_TRUE(static_cast<bool>(copy.GetChecksum()));
-  EXPECT_EQ(file_spec.GetChecksum(), copy.GetChecksum());
-}