[clangd] NFC: Add using directives to avoid spelling out llvm::sys::path

[kbobyrev]

Summary

llvm::sys::path is used a lot in the remote index marshalling code. We can save space by avoiding spelling it out explicitly for most functions and times.

[ArcsinX]

clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp

61–62

llvm::sys::path::is_absolute(RemoteIndexRoot) => is_absolute(RemoteIndexRoot) ?

68–69

llvm::sys::path::is_absolute(LocalIndexRoot) => is_absolute(LocalIndexRoot) ?

318

llvm::sys::path::convert_to_slash(RelativePath) => convert_to_slash(RelativePath) ?

[kbobyrev]

The ones you proposed would not work without using llvm::sys::path::* which I avoid :( They have to have an argument from llvm::sys::path:: in order for the lookup to be performed correctly.

[ArcsinX]

In D90016#2349436, @kbobyrev wrote:

The ones you proposed would not work without using llvm::sys::path::* which I avoid :( They have to have an argument from llvm::sys::path:: in order for the lookup to be performed correctly.

Ah, sorry, got it.

[kadircet]

I am not really keen about trusting ADL for these, as it makes the code less explicit for reader, especially as it is not something we commonly depend on clangd so readers/reviewers would not be expecting that. So I believe we should not stray away from the general project style in here.

Though I am a fan of not spelling llvm::sys::path bazillion times. What about having either a using namepsace for it or, using declarations for the individual symbols we are using (e.g. Style, covnert_to_slash and is_absolute)? This would also help with not spelling qualifiers over and over again in the argument names.

[kbobyrev]

Add using directives for llvm::sys::path functions used more than once.

[kadircet]

clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp

31

nit: I would move those into clang::clangd::remote (i.e. between namespace remote { and namespace {.

[kbobyrev]

Resolve the comment

clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp

 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
 namespace clang {
 namespace clangd {
 namespace remote {
 
+using llvm::sys::path::append;
+using llvm::sys::path::convert_to_slash;
+using llvm::sys::path::is_absolute;
+using llvm::sys::path::replace_path_prefix;
+using llvm::sys::path::Style;
+
 namespace {
 
 template <typename IDRange>
 llvm::Expected<llvm::DenseSet<SymbolID>> getIDs(IDRange IDs) {
   llvm::DenseSet<SymbolID> Result;
   for (const auto &ID : IDs) {
     auto SID = SymbolID::fromStr(StringRef(ID));
     if (!SID)
       return SID.takeError();
     Result.insert(*SID);
   }
   return Result;
 }
 
 } // namespace
 
 Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
                        llvm::StringRef LocalIndexRoot)
     : Strings(Arena) {
-  llvm::StringRef PosixSeparator =
-      llvm::sys::path::get_separator(llvm::sys::path::Style::posix);
+  llvm::StringRef PosixSeparator = get_separator(Style::posix);
   if (!RemoteIndexRoot.empty()) {
-    assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
-    this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
-        RemoteIndexRoot, llvm::sys::path::Style::windows);
+    assert(is_absolute(RemoteIndexRoot));
+    this->RemoteIndexRoot = convert_to_slash(RemoteIndexRoot, Style::windows);
     llvm::StringRef Path(this->RemoteIndexRoot);
-    if (!is_separator(this->RemoteIndexRoot.back(),
-                      llvm::sys::path::Style::posix))
+    if (!is_separator(this->RemoteIndexRoot.back(), Style::posix))
       this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
-    assert(llvm::sys::path::is_absolute(LocalIndexRoot));
-    this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
-        LocalIndexRoot, llvm::sys::path::Style::windows);
+    assert(is_absolute(LocalIndexRoot));
+    this->LocalIndexRoot = convert_to_slash(LocalIndexRoot, Style::windows);
     llvm::StringRef Path(this->LocalIndexRoot);
-    if (!is_separator(this->LocalIndexRoot.back(),
-                      llvm::sys::path::Style::posix))
+    if (!is_separator(this->LocalIndexRoot.back(), Style::posix))
       this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
 
 llvm::Expected<clangd::LookupRequest>
 Marshaller::fromProtobuf(const v1::LookupRequest *Message) {
   clangd::LookupRequest Req;
Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) {
   for (const auto &Scope : Message->scopes())
     Result.Scopes.push_back(Scope);
   Result.AnyScope = Message->any_scope();
   if (Message->limit())
     Result.Limit = Message->limit();
   Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
   for (const auto &Path : Message->proximity_paths()) {
     llvm::SmallString<256> LocalPath = llvm::StringRef(RemoteIndexRoot);
-    llvm::sys::path::append(LocalPath, Path);
+    append(LocalPath, Path);
     // FuzzyFindRequest requires proximity paths to have platform-native format
     // in order for SymbolIndex to process the query correctly.
     llvm::sys::path::native(LocalPath);
     Result.ProximityPaths.push_back(std::string(LocalPath));
   }
   for (const auto &Type : Message->preferred_types())
     Result.ProximityPaths.push_back(Type);
   return Result;
Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
   for (const auto &Scope : From.Scopes)
     RPCRequest.add_scopes(Scope);
   RPCRequest.set_any_scope(From.AnyScope);
   if (From.Limit)
     RPCRequest.set_limit(*From.Limit);
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths) {
     llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-    if (llvm::sys::path::replace_path_prefix(RelativePath, LocalIndexRoot, ""))
-      RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
-          RelativePath, llvm::sys::path::Style::windows));
+    if (replace_path_prefix(RelativePath, LocalIndexRoot, ""))
+      RPCRequest.add_proximity_paths(
+          convert_to_slash(RelativePath, Style::windows));
   }
   for (const auto &Type : From.PreferredTypes)
     RPCRequest.add_preferred_types(Type);
   return RPCRequest;
 }
 
 v1::RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) {
   v1::RefsRequest RPCRequest;
if (!SerializedObject)
     return SerializedObject.takeError();
   *Result.mutable_object() = *SerializedObject;
   return Result;
 }
 
 llvm::Expected<std::string>
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
   assert(!LocalIndexRoot.empty());
-  assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
+  assert(RelativePath == convert_to_slash(RelativePath));
   if (RelativePath.empty())
     return error("Empty relative path.");
-  if (llvm::sys::path::is_absolute(RelativePath, llvm::sys::path::Style::posix))
+  if (is_absolute(RelativePath, Style::posix))
     return error("RelativePath '{0}' is absolute.", RelativePath);
   llvm::SmallString<256> FullPath = llvm::StringRef(LocalIndexRoot);
-  llvm::sys::path::append(FullPath, RelativePath);
+  append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
   return Result.toString();
 }
 
 llvm::Expected<std::string> Marshaller::uriToRelativePath(llvm::StringRef URI) {
   assert(!RemoteIndexRoot.empty());
   auto ParsedURI = URI::parse(URI);
   if (!ParsedURI)
     return ParsedURI.takeError();
   if (ParsedURI->scheme() != "file")
     return error("Can not use URI schemes other than file, given: '{0}'.", URI);
   llvm::SmallString<256> Result = ParsedURI->body();
   llvm::StringRef Path(Result);
   // Check for Windows paths (URI=file:///X:/path => Body=/X:/path)
-  if (llvm::sys::path::is_absolute(Path.substr(1),
-                                   llvm::sys::path::Style::windows))
+  if (is_absolute(Path.substr(1), Style::windows))
     Result = Path.drop_front();
-  if (!llvm::sys::path::replace_path_prefix(Result, RemoteIndexRoot, ""))
+  if (!replace_path_prefix(Result, RemoteIndexRoot, ""))
     return error("File path '{0}' doesn't start with '{1}'.", Result.str(),
                  RemoteIndexRoot);
-  assert(Result == llvm::sys::path::convert_to_slash(
-                       Result, llvm::sys::path::Style::windows));
+  assert(Result == convert_to_slash(Result, Style::windows));
   return std::string(Result);
 }
 
 clangd::SymbolLocation::Position
 Marshaller::fromProtobuf(const v1::Position &Message) {
   clangd::SymbolLocation::Position Result;
   Result.setColumn(static_cast<uint32_t>(Message.column()));
   Result.setLine(static_cast<uint32_t>(Message.line()));