JDevlieghere updated this revision to Diff 534667.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.
Remove unused `Resolver`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153726/new/
https://reviews.llvm.org/D153726
Files:
lldb/include/lldb/Core/SourceManager.h
lldb/include/lldb/Host/FileSystem.h
lldb/source/Core/SourceManager.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/unittests/Core/SourceManagerTest.cpp
Index: lldb/unittests/Core/SourceManagerTest.cpp
===================================================================
--- lldb/unittests/Core/SourceManagerTest.cpp
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -10,12 +10,17 @@
#include "lldb/Host/FileSystem.h"
#include "gtest/gtest.h"
+#include "TestingSupport/MockTildeExpressionResolver.h"
+
using namespace lldb;
using namespace lldb_private;
class SourceFileCache : public ::testing::Test {
public:
- void SetUp() override { FileSystem::Initialize(); }
+ void SetUp() override {
+ FileSystem::Initialize(std::unique_ptr<TildeExpressionResolver>(
+ new MockTildeExpressionResolver("Jonas", "/jonas")));
+ }
void TearDown() override { FileSystem::Terminate(); }
};
@@ -26,7 +31,7 @@
FileSpec foo_file_spec("foo");
auto foo_file_sp =
std::make_shared<SourceManager::File>(foo_file_spec, nullptr);
- cache.AddSourceFile(foo_file_sp);
+ cache.AddSourceFile(foo_file_spec, foo_file_sp);
// Query: foo, expect found.
FileSpec another_foo_file_spec("foo");
@@ -40,9 +45,32 @@
FileSpec foo_file_spec("foo");
auto foo_file_sp =
std::make_shared<SourceManager::File>(foo_file_spec, nullptr);
- cache.AddSourceFile(foo_file_sp);
+ cache.AddSourceFile(foo_file_spec, foo_file_sp);
// Query: bar, expect not found.
FileSpec bar_file_spec("bar");
ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
}
+
+TEST_F(SourceFileCache, FindSourceFileByUnresolvedPath) {
+ SourceManager::SourceFileCache cache;
+
+ FileSpec foo_file_spec("~/foo");
+
+ // Mimic the resolution in SourceManager::GetFile.
+ FileSpec resolved_foo_file_spec = foo_file_spec;
+ FileSystem::Instance().Resolve(resolved_foo_file_spec);
+
+ // Create the file with the resolved file spec.
+ auto foo_file_sp =
+ std::make_shared<SourceManager::File>(resolved_foo_file_spec, nullptr);
+
+ // Cache the result with the unresolved file spec.
+ cache.AddSourceFile(foo_file_spec, foo_file_sp);
+
+ // Query the unresolved path.
+ EXPECT_EQ(cache.FindSourceFile(FileSpec("~/foo")), foo_file_sp);
+
+ // Query the resolved path.
+ EXPECT_EQ(cache.FindSourceFile(FileSpec("/jonas/foo")), foo_file_sp);
+}
Index: lldb/source/Host/common/FileSystem.cpp
===================================================================
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -9,8 +9,6 @@
#include "lldb/Host/FileSystem.h"
#include "lldb/Utility/DataBufferLLVM.h"
-#include "lldb/Utility/LLDBAssert.h"
-#include "lldb/Utility/TildeExpressionResolver.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Errno.h"
@@ -46,16 +44,6 @@
FileSystem &FileSystem::Instance() { return *InstanceImpl(); }
-void FileSystem::Initialize() {
- lldbassert(!InstanceImpl() && "Already initialized.");
- InstanceImpl().emplace();
-}
-
-void FileSystem::Initialize(IntrusiveRefCntPtr<vfs::FileSystem> fs) {
- lldbassert(!InstanceImpl() && "Already initialized.");
- InstanceImpl().emplace(fs);
-}
-
void FileSystem::Terminate() {
lldbassert(InstanceImpl() && "Already terminated.");
InstanceImpl().reset();
@@ -239,9 +227,9 @@
// Resolve tilde in path.
SmallString<128> resolved(path.begin(), path.end());
- StandardTildeExpressionResolver Resolver;
- Resolver.ResolveFullPath(llvm::StringRef(path.begin(), path.size()),
- resolved);
+ assert(m_tilde_resolver && "must initialize tilde resolver in constructor");
+ m_tilde_resolver->ResolveFullPath(llvm::StringRef(path.begin(), path.size()),
+ resolved);
// Try making the path absolute if it exists.
SmallString<128> absolute(resolved.begin(), resolved.end());
Index: lldb/source/Core/SourceManager.cpp
===================================================================
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -75,13 +75,10 @@
if (!file_spec)
return nullptr;
- FileSpec resolved_fspec = file_spec;
- resolve_tilde(resolved_fspec);
-
DebuggerSP debugger_sp(m_debugger_wp.lock());
FileSP file_sp;
if (debugger_sp && debugger_sp->GetUseSourceCache())
- file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(resolved_fspec);
+ file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
TargetSP target_sp(m_target_wp.lock());
@@ -99,12 +96,12 @@
// If file_sp is no good or it points to a non-existent file, reset it.
if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
if (target_sp)
- file_sp = std::make_shared<File>(resolved_fspec, target_sp.get());
+ file_sp = std::make_shared<File>(file_spec, target_sp.get());
else
- file_sp = std::make_shared<File>(resolved_fspec, debugger_sp);
+ file_sp = std::make_shared<File>(file_spec, debugger_sp);
if (debugger_sp && debugger_sp->GetUseSourceCache())
- debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
+ debugger_sp->GetSourceFileCache().AddSourceFile(file_spec, file_sp);
}
return file_sp;
}
@@ -393,15 +390,13 @@
SourceManager::File::File(const FileSpec &file_spec,
lldb::DebuggerSP debugger_sp)
- : m_file_spec_orig(file_spec), m_file_spec(file_spec),
- m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
+ : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
m_debugger_wp(debugger_sp) {
CommonInitializer(file_spec, nullptr);
}
SourceManager::File::File(const FileSpec &file_spec, Target *target)
- : m_file_spec_orig(file_spec), m_file_spec(file_spec),
- m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
+ : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
m_debugger_wp(target ? target->GetDebugger().shared_from_this()
: DebuggerSP()) {
CommonInitializer(file_spec, target);
@@ -409,13 +404,16 @@
void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
Target *target) {
+ // Set the file and update the modification time.
+ SetFileSpec(file_spec);
+
+ // File doesn't exist.
if (m_mod_time == llvm::sys::TimePoint<>()) {
if (target) {
m_source_map_mod_id = target->GetSourcePathMap().GetModificationID();
+ // If this is just a file name, try finding it in the target.
if (!file_spec.GetDirectory() && file_spec.GetFilename()) {
- // If this is just a file name, lets see if we can find it in the
- // target:
bool check_inlines = false;
SymbolContextList sc_list;
size_t num_matches =
@@ -443,13 +441,12 @@
SymbolContext sc;
sc_list.GetContextAtIndex(0, sc);
if (sc.comp_unit)
- m_file_spec = sc.comp_unit->GetPrimaryFile();
- m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+ SetFileSpec(sc.comp_unit->GetPrimaryFile());
}
}
}
- resolve_tilde(m_file_spec);
- // Try remapping if m_file_spec does not correspond to an existing file.
+
+ // Try remapping the file if it doesn't exist.
if (!FileSystem::Instance().Exists(m_file_spec)) {
// Check target specific source remappings (i.e., the
// target.source-map setting), then fall back to the module
@@ -460,18 +457,23 @@
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);
- }
+ if (remapped)
+ SetFileSpec(*remapped);
}
}
}
+ // If the file exists, read in the data.
if (m_mod_time != llvm::sys::TimePoint<>())
m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
}
+void SourceManager::File::SetFileSpec(FileSpec file_spec) {
+ resolve_tilde(file_spec);
+ m_file_spec = std::move(file_spec);
+ m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+}
+
uint32_t SourceManager::File::GetLineOffset(uint32_t line) {
if (line == 0)
return UINT32_MAX;
@@ -708,12 +710,22 @@
return true;
}
-void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
- FileSpec file_spec = file_sp->GetFileSpec();
+void SourceManager::SourceFileCache::AddSourceFile(const FileSpec &file_spec,
+ FileSP file_sp) {
+ assert(file_sp && "invalid FileSP");
+
+ AddSourceFileImpl(file_spec, file_sp);
+ const FileSpec &resolved_file_spec = file_sp->GetFileSpec();
+ if (file_spec != resolved_file_spec)
+ AddSourceFileImpl(file_sp->GetFileSpec(), file_sp);
+}
+
+void SourceManager::SourceFileCache::AddSourceFileImpl(
+ const FileSpec &file_spec, FileSP file_sp) {
FileCache::iterator pos = m_file_cache.find(file_spec);
- if (pos == m_file_cache.end())
+ if (pos == m_file_cache.end()) {
m_file_cache[file_spec] = file_sp;
- else {
+ } else {
if (file_sp != pos->second)
m_file_cache[file_spec] = file_sp;
}
Index: lldb/include/lldb/Host/FileSystem.h
===================================================================
--- lldb/include/lldb/Host/FileSystem.h
+++ lldb/include/lldb/Host/FileSystem.h
@@ -12,7 +12,9 @@
#include "lldb/Host/File.h"
#include "lldb/Utility/DataBuffer.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/TildeExpressionResolver.h"
#include "llvm/Support/Chrono.h"
#include "llvm/Support/VirtualFileSystem.h"
@@ -30,17 +32,25 @@
static const char *DEV_NULL;
static const char *PATH_CONVERSION_ERROR;
- FileSystem() : m_fs(llvm::vfs::getRealFileSystem()) {}
+ FileSystem()
+ : m_fs(llvm::vfs::getRealFileSystem()),
+ m_tilde_resolver(std::make_unique<StandardTildeExpressionResolver>()) {}
FileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs)
- : m_fs(std::move(fs)) {}
+ : m_fs(std::move(fs)),
+ m_tilde_resolver(std::make_unique<StandardTildeExpressionResolver>()) {}
+ FileSystem(std::unique_ptr<TildeExpressionResolver> tilde_resolver)
+ : m_fs(llvm::vfs::getRealFileSystem()),
+ m_tilde_resolver(std::move(tilde_resolver)) {}
FileSystem(const FileSystem &fs) = delete;
FileSystem &operator=(const FileSystem &fs) = delete;
static FileSystem &Instance();
- static void Initialize();
- static void Initialize(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
+ template <class... T> static void Initialize(T &&...t) {
+ lldbassert(!InstanceImpl() && "Already initialized.");
+ InstanceImpl().emplace(std::forward<T>(t)...);
+ }
static void Terminate();
Status Symlink(const FileSpec &src, const FileSpec &dst);
@@ -197,6 +207,7 @@
private:
static std::optional<FileSystem> &InstanceImpl();
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
+ std::unique_ptr<TildeExpressionResolver> m_tilde_resolver;
std::string m_home_directory;
};
} // namespace lldb_private
Index: lldb/include/lldb/Core/SourceManager.h
===================================================================
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -68,6 +68,9 @@
llvm::sys::TimePoint<> GetTimestamp() const { return m_mod_time; }
protected:
+ /// Set file and update modification time.
+ void SetFileSpec(FileSpec file_spec);
+
bool CalculateLineOffsets(uint32_t line = UINT32_MAX);
FileSpec m_file_spec_orig; // The original file spec that was used (can be
@@ -101,7 +104,7 @@
SourceFileCache() = default;
~SourceFileCache() = default;
- void AddSourceFile(const FileSP &file_sp);
+ void AddSourceFile(const FileSpec &file_spec, FileSP file_sp);
FileSP FindSourceFile(const FileSpec &file_spec) const;
// Removes all elements from the cache.
@@ -109,7 +112,9 @@
void Dump(Stream &stream) const;
- protected:
+ private:
+ void AddSourceFileImpl(const FileSpec &file_spec, FileSP file_sp);
+
typedef std::map<FileSpec, FileSP> FileCache;
FileCache m_file_cache;
};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
