[Lldb-commits] [PATCH] D130396: Cache the value for absolute path in FileSpec.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdabe877248b8: Cache the value for absolute path in FileSpec. 
(authored by clayborg).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130396/new/

https://reviews.llvm.org/D130396

Files:
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp


Index: lldb/unittests/Utility/FileSpecTest.cpp
===================================================================
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -450,3 +450,29 @@
   EXPECT_FALSE(FileSpec(""));
   EXPECT_TRUE(FileSpec("/foo/bar"));
 }
+
+
+TEST(FileSpecTest, TestAbsoluteCaching) {
+  // Test that if we modify a path that we recalculate if a path is relative
+  // or absolute correctly. The test below calls set accessors and functions
+  // that change the path and verifies that the FileSpec::IsAbsolute() returns
+  // the correct value.
+  FileSpec file = PosixSpec("/tmp/a");
+  EXPECT_TRUE(file.IsAbsolute());
+  file.ClearDirectory();
+  EXPECT_FALSE(file.IsAbsolute());
+  file.SetDirectory("/tmp");
+  EXPECT_TRUE(file.IsAbsolute());
+  file.SetDirectory(".");
+  EXPECT_FALSE(file.IsAbsolute());
+  file.SetPath("/log.txt");
+  EXPECT_TRUE(file.IsAbsolute());
+  file.RemoveLastPathComponent();
+  EXPECT_TRUE(file.IsAbsolute());
+  file.ClearFilename();
+  EXPECT_FALSE(file.IsAbsolute());
+  file.AppendPathComponent("foo.txt");
+  EXPECT_FALSE(file.IsAbsolute());
+  file.PrependPathComponent("/tmp");
+  EXPECT_TRUE(file.IsAbsolute());
+}
Index: lldb/source/Utility/FileSpec.cpp
===================================================================
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -170,9 +170,7 @@
 // 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) {
-  m_filename.Clear();
-  m_directory.Clear();
-  m_is_resolved = false;
+  Clear();
   m_style = (style == Style::native) ? GetNativeStyle() : style;
 
   if (pathname.empty())
@@ -259,6 +257,7 @@
 void FileSpec::Clear() {
   m_directory.Clear();
   m_filename.Clear();
+  PathWasModified();
 }
 
 // Compare two FileSpec objects. If "full" is true, then both the directory and
@@ -332,26 +331,32 @@
 
 void FileSpec::SetDirectory(ConstString directory) {
   m_directory = directory;
+  PathWasModified();
 }
 
 void FileSpec::SetDirectory(llvm::StringRef directory) {
   m_directory = ConstString(directory);
+  PathWasModified();
 }
 
 void FileSpec::SetFilename(ConstString filename) {
   m_filename = filename;
+  PathWasModified();
 }
 
 void FileSpec::SetFilename(llvm::StringRef filename) {
   m_filename = ConstString(filename);
+  PathWasModified();
 }
 
 void FileSpec::ClearFilename() {
   m_filename.Clear();
+  PathWasModified();
 }
 
 void FileSpec::ClearDirectory() {
   m_directory.Clear();
+  PathWasModified();
 }
 
 // Extract the directory and path into a fixed buffer. This is needed as the
@@ -488,18 +493,22 @@
 }
 
 bool FileSpec::IsAbsolute() const {
-llvm::SmallString<64> current_path;
-  GetPath(current_path, false);
+  // Check if we have cached if this path is absolute to avoid recalculating.
+  if (m_absolute != Absolute::Calculate)
+    return m_absolute == Absolute::Yes;
 
-  // Early return if the path is empty.
-  if (current_path.empty())
-    return false;
+  m_absolute = Absolute::No;
 
-  // We consider paths starting with ~ to be absolute.
-  if (current_path[0] == '~')
-    return true;
+  llvm::SmallString<64> path;
+  GetPath(path, false);
+
+  if (!path.empty()) {
+    // We consider paths starting with ~ to be absolute.
+    if (path[0] == '~' || llvm::sys::path::is_absolute(path, m_style))
+      m_absolute = Absolute::Yes;
+  }
 
-  return llvm::sys::path::is_absolute(current_path, m_style);
+  return m_absolute == Absolute::Yes;
 }
 
 void FileSpec::MakeAbsolute(const FileSpec &dir) {
Index: lldb/include/lldb/Utility/FileSpec.h
===================================================================
--- lldb/include/lldb/Utility/FileSpec.h
+++ lldb/include/lldb/Utility/FileSpec.h
@@ -416,10 +416,24 @@
   // 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_is_resolved = false;
+    m_absolute = Absolute::Calculate;
+  }
+
+  enum class Absolute : uint8_t {
+    Calculate,
+    Yes,
+    No
+  };
+
   // Member variables
   ConstString m_directory;            ///< The uniqued directory path
   ConstString m_filename;             ///< The uniqued filename path
   mutable bool m_is_resolved = false; ///< True if this path has been resolved.
+  mutable Absolute m_absolute = Absolute::Calculate; ///< Cache absoluteness.
   Style m_style; ///< The syntax that this path uses (e.g. Windows / Posix)
 };
 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits