[clangd] Expose actOnAllPArentDirectories helper

[kadircet]

Summary

Will be used in other components that need ancestor traversal.

[sammccall]

clang-tools-extra/clangd/support/Path.h

33

The signature is a bit weird here,

• prone to boolean-sense mistakes
• it's not obvious it only works on absolute paths
• the loop isn't hard to write

maybe we should expose absoluteParent instead?

[kadircet]

maybe we should expose absoluteParent instead?

doing that instead. also updating tidyprovider and configyamlparser to make use of this helper now, PTAL.

[kadircet]

• Expose absoluteParent instead of whole traverse action
• Use helper in existing places that use path::parent_path for traversal

[sammccall]

Thanks!

clang-tools-extra/clangd/ConfigProvider.cpp

101 ↗

(On Diff #323820)

absoluteParent?

I think this can be moved into the loop init now, it's not used anywhere else

clang-tools-extra/clangd/support/Path.h

31

Maybe add a hint why we use this...

"unlike sys::parent_path, does not consider C: a parent of C:\"

[kadircet]

• Update comment.
• Get rid of a parent_path usage.

[kadircet]

• Get rid of parent_path usage for real

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

 #include <condition_variable>
 #include <mutex>
 #include <string>
 #include <tuple>
 #include <vector>
 
 namespace clang {
 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,
-                               llvm::function_ref<bool(PathRef)> Action) {
-  for (auto Path = absoluteParent(FileName); !Path.empty() && !Action(Path);
-       Path = absoluteParent(Path))
-    ;
-}
-
-} // namespace
-
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
   std::vector<std::string> Argv = {"clang"};
   // Clang treats .h files as C by default and files without extension as linker
   // input, resulting in unhelpful diagnostics.
   // Parsing as Objective C++ is friendly to more cases.
   auto FileExtension = llvm::sys::path::extension(File);
   if (FileExtension.empty() || FileExtension == ".h")

clang-tools-extra/clangd/support/Path.h

 using PathRef = llvm::StringRef;
 
 // For platforms where paths are case-insensitive (but case-preserving),
 // we need to do case-insensitive comparisons and use lowercase keys.
 // FIXME: Make Path a real class with desired semantics instead.
 std::string maybeCaseFoldPath(PathRef Path);
 bool pathEqual(PathRef, PathRef);
 
+// 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,
+                               llvm::function_ref<bool(PathRef)> Action);
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/support/Path.cpp

 //===--- Path.cpp -------------------------------------------*- C++-*------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "support/Path.h"
+#include "llvm/Support/Path.h"
+
 namespace clang {
 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;
+}
+
+} // namespace
 
 std::string maybeCaseFoldPath(PathRef Path) {
 #if defined(_WIN32) || defined(__APPLE__)
   return Path.lower();
 #else
   return std::string(Path);
 #endif
 }
 
 bool pathEqual(PathRef A, PathRef B) {
 #if defined(_WIN32) || defined(__APPLE__)
   return A.equals_lower(B);
 #else
   return A == B;
 #endif
 }
 
+void actOnAllParentDirectories(PathRef FileName,
+                               llvm::function_ref<bool(PathRef)> Action) {
+  for (auto Path = absoluteParent(FileName); !Path.empty() && !Action(Path);
+       Path = absoluteParent(Path))
+    ;
+}
 } // namespace clangd
 } // namespace clang