diff --git a/clang-tools-extra/clangd/ConfigProvider.cpp b/clang-tools-extra/clangd/ConfigProvider.cpp --- a/clang-tools-extra/clangd/ConfigProvider.cpp +++ b/clang-tools-extra/clangd/ConfigProvider.cpp @@ -10,8 +10,10 @@ #include "Config.h" #include "ConfigFragment.h" #include "support/FileCache.h" +#include "support/Path.h" #include "support/ThreadsafeFS.h" #include "support/Trace.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" @@ -96,16 +98,10 @@ return {}; // Compute absolute paths to all ancestors (substrings of P.Path). - llvm::StringRef Parent = path::parent_path(P.Path); llvm::SmallVector Ancestors; - for (auto I = path::begin(Parent, path::Style::posix), - E = path::end(Parent); - I != E; ++I) { - // Avoid weird non-substring cases like phantom "." components. - // In practice, Component is a substring for all "normal" ancestors. - if (I->end() < Parent.begin() || I->end() > Parent.end()) - continue; - Ancestors.emplace_back(Parent.begin(), I->end() - Parent.begin()); + for (auto Ancestor = absoluteParent(P.Path); !Ancestor.empty(); + Ancestor = absoluteParent(Ancestor)) { + Ancestors.emplace_back(Ancestor); } // Ensure corresponding cache entries exist in the map. llvm::SmallVector Caches; @@ -127,7 +123,7 @@ // Finally query each individual file. // This will take a (per-file) lock for each file that actually exists. std::vector Result; - for (FileConfigCache *Cache : Caches) + for (FileConfigCache *Cache : llvm::reverse(Caches)) Cache->get(FS, DC, P.FreshTime, Result); return Result; }; diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -43,21 +43,6 @@ namespace clangd { namespace { -// Variant of parent_path that operates only on absolute paths. -PathRef absoluteParent(PathRef Path) { - assert(llvm::sys::path::is_absolute(Path)); -#if defined(_WIN32) - // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative. - // This unhelpful behavior seems to have been inherited from boost. - if (llvm::sys::path::relative_path(Path).empty()) { - return PathRef(); - } -#endif - PathRef Result = llvm::sys::path::parent_path(Path); - assert(Result.empty() || llvm::sys::path::is_absolute(Result)); - return Result; -} - // Runs the given action on all parent directories of filename, starting from // deepest directory and going up to root. Stops whenever action succeeds. void actOnAllParentDirectories(PathRef FileName, diff --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp --- a/clang-tools-extra/clangd/TidyProvider.cpp +++ b/clang-tools-extra/clangd/TidyProvider.cpp @@ -11,6 +11,7 @@ #include "Config.h" #include "support/FileCache.h" #include "support/Logger.h" +#include "support/Path.h" #include "support/ThreadsafeFS.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" @@ -102,23 +103,12 @@ // Compute absolute paths to all ancestors (substrings of P.Path). // Ensure cache entries for each ancestor exist in the map. - llvm::StringRef Parent = path::parent_path(AbsPath); llvm::SmallVector Caches; { std::lock_guard Lock(Mu); - for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) { - assert(I->end() >= Parent.begin() && I->end() <= Parent.end() && - "Canonical path components should be substrings"); - llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin()); -#ifdef _WIN32 - // C:\ is an ancestor, but skip its (relative!) parent C:. - if (Ancestor.size() == 2 && Ancestor.back() == ':') - continue; -#endif - assert(path::is_absolute(Ancestor)); - + for (auto Ancestor = absoluteParent(AbsPath); !Ancestor.empty(); + Ancestor = absoluteParent(Ancestor)) { auto It = Cache.find(Ancestor); - // Assemble the actual config file path only if needed. if (It == Cache.end()) { llvm::SmallString<256> ConfigPath = Ancestor; diff --git a/clang-tools-extra/clangd/support/Path.h b/clang-tools-extra/clangd/support/Path.h --- a/clang-tools-extra/clangd/support/Path.h +++ b/clang-tools-extra/clangd/support/Path.h @@ -40,6 +40,10 @@ bool pathStartsWith( PathRef Ancestor, PathRef Path, llvm::sys::path::Style Style = llvm::sys::path::Style::native); + +/// Variant of parent_path that operates only on absolute paths. +/// Unlike parent_path doesn't consider C: a parent of C:\. +PathRef absoluteParent(PathRef Path); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/support/Path.cpp b/clang-tools-extra/clangd/support/Path.cpp --- a/clang-tools-extra/clangd/support/Path.cpp +++ b/clang-tools-extra/clangd/support/Path.cpp @@ -19,6 +19,20 @@ bool pathEqual(PathRef A, PathRef B) { return A == B; } #endif // CLANGD_PATH_CASE_INSENSITIVE +PathRef absoluteParent(PathRef Path) { + assert(llvm::sys::path::is_absolute(Path)); +#if defined(_WIN32) + // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative. + // This unhelpful behavior seems to have been inherited from boost. + if (llvm::sys::path::relative_path(Path).empty()) { + return PathRef(); + } +#endif + PathRef Result = llvm::sys::path::parent_path(Path); + assert(Result.empty() || llvm::sys::path::is_absolute(Result)); + return Result; +} + bool pathStartsWith(PathRef Ancestor, PathRef Path, llvm::sys::path::Style Style) { assert(llvm::sys::path::is_absolute(Ancestor) &&