[clangd] Simplify the callside of URI::resolve, NFC.
authorHaojian Wu <hokein@google.com>
Mon, 23 Sep 2019 14:39:37 +0000 (14:39 +0000)
committerHaojian Wu <hokein@google.com>
Mon, 23 Sep 2019 14:39:37 +0000 (14:39 +0000)
Summary:
- Add a overrloded URI::resolve, which accepts a string URI;
- also fixed some callside that don't check the error;

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D67916

llvm-svn: 372617

clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/FindSymbols.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/URI.cpp
clang-tools-extra/clangd/URI.h
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp

index b96636aaf63be76dbde0eb5019483f71463511a7..2b6c3b462709d490da6129133a6c482a13ed3c2c 100644 (file)
@@ -318,11 +318,8 @@ struct CodeCompletionBuilder {
     // Turn absolute path into a literal string that can be #included.
     auto Inserted = [&](llvm::StringRef Header)
         -> llvm::Expected<std::pair<std::string, bool>> {
-      auto DeclaringURI =
-          URI::parse(C.IndexResult->CanonicalDeclaration.FileURI);
-      if (!DeclaringURI)
-        return DeclaringURI.takeError();
-      auto ResolvedDeclaring = URI::resolve(*DeclaringURI, FileName);
+      auto ResolvedDeclaring =
+          URI::resolve(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
       if (!ResolvedDeclaring)
         return ResolvedDeclaring.takeError();
       auto ResolvedInserted = toHeaderFile(Header, FileName);
index 287a8a4df0b43c286183102b3efbb284a06d85e6..619f1a5cfdbcdc4545eea5cb641b8063e3c0fff0 100644 (file)
@@ -43,18 +43,11 @@ llvm::Expected<Location> symbolToLocation(const Symbol &Sym,
                                           llvm::StringRef HintPath) {
   // Prefer the definition over e.g. a function declaration in a header
   auto &CD = Sym.Definition ? Sym.Definition : Sym.CanonicalDeclaration;
-  auto Uri = URI::parse(CD.FileURI);
-  if (!Uri) {
-    return llvm::make_error<llvm::StringError>(
-        formatv("Could not parse URI '{0}' for symbol '{1}'.", CD.FileURI,
-                Sym.Name),
-        llvm::inconvertibleErrorCode());
-  }
-  auto Path = URI::resolve(*Uri, HintPath);
+  auto Path = URI::resolve(CD.FileURI, HintPath);
   if (!Path) {
     return llvm::make_error<llvm::StringError>(
-        formatv("Could not resolve path for URI '{0}' for symbol '{1}'.",
-                Uri->toString(), Sym.Name),
+        formatv("Could not resolve path for symbol '{0}': {1}",
+                Sym.Name, llvm::toString(Path.takeError())),
         llvm::inconvertibleErrorCode());
   }
   Location L;
index 081dad83583b4b0813c4a2fbc9d840cc5366c28a..93dee2eabfc6dfe69e677b467530fc741c2fd48b 100644 (file)
@@ -144,10 +144,8 @@ std::vector<Fix> IncludeFixer::fixIncompleteType(const Type &T) const {
 std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
   auto Inserted = [&](const Symbol &Sym, llvm::StringRef Header)
       -> llvm::Expected<std::pair<std::string, bool>> {
-    auto DeclaringURI = URI::parse(Sym.CanonicalDeclaration.FileURI);
-    if (!DeclaringURI)
-      return DeclaringURI.takeError();
-    auto ResolvedDeclaring = URI::resolve(*DeclaringURI, File);
+    auto ResolvedDeclaring =
+        URI::resolve(Sym.CanonicalDeclaration.FileURI, File);
     if (!ResolvedDeclaring)
       return ResolvedDeclaring.takeError();
     auto ResolvedInserted = toHeaderFile(Header, File);
index 9f5c92b684ff96ddc48ee4437123eae7802a607b..f8a9f59ca13781ca91e4c7d35bbfe1d4ddc7e2a2 100644 (file)
@@ -183,6 +183,17 @@ llvm::Expected<URI> URI::parse(llvm::StringRef OrigUri) {
   return U;
 }
 
+llvm::Expected<std::string> URI::resolve(llvm::StringRef FileURI,
+                                         llvm::StringRef HintPath) {
+  auto Uri = URI::parse(FileURI);
+  if (!Uri)
+    return Uri.takeError();
+  auto Path = URI::resolve(*Uri, HintPath);
+  if (!Path)
+    return Path.takeError();
+  return *Path;
+}
+
 llvm::Expected<URI> URI::create(llvm::StringRef AbsolutePath,
                                 llvm::StringRef Scheme) {
   if (!llvm::sys::path::is_absolute(AbsolutePath))
index 110b39af663cbb80756430924dfbf080869ea152..5f120ef9e386fc688a352a4af316467efacaa460 100644 (file)
@@ -63,6 +63,10 @@ public:
   static llvm::Expected<std::string> resolve(const URI &U,
                                              llvm::StringRef HintPath = "");
 
+  /// Same as above, in addition it parses the \p FileURI using URI::parse.
+  static llvm::Expected<std::string> resolve(llvm::StringRef FileURI,
+                                             llvm::StringRef HintPath = "");
+
   /// Resolves \p AbsPath into a canonical path of its URI, by converting
   /// \p AbsPath to URI and resolving the URI to get th canonical path.
   /// This ensures that paths with the same URI are resolved into consistent
index 9ea21fdf6c59045ba5fce9afc0dea8d1a87f236d..ff6a9e52268431dc9c3fdcf6f6d2c7f6ce455cd6 100644 (file)
@@ -69,13 +69,7 @@ public:
   llvm::StringRef resolve(llvm::StringRef FileURI) {
     auto I = URIToPathCache.try_emplace(FileURI);
     if (I.second) {
-      auto U = URI::parse(FileURI);
-      if (!U) {
-        elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
-        assert(false && "Failed to parse URI");
-        return "";
-      }
-      auto Path = URI::resolve(*U, HintPath);
+      auto Path = URI::resolve(FileURI, HintPath);
       if (!Path) {
         elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
         assert(false && "Failed to resolve URI");
index 585d36716d29646bb370fe42f408b417868d7f0a..ff690f90257331bf90d0600b1c91576739065775 100644 (file)
@@ -24,16 +24,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-llvm::Optional<Path> uriToAbsolutePath(llvm::StringRef URI, PathRef HintPath) {
-  auto U = URI::parse(URI);
-  if (!U)
-    return llvm::None;
-  auto AbsolutePath = URI::resolve(*U, HintPath);
-  if (!AbsolutePath)
-    return llvm::None;
-  return *AbsolutePath;
-}
-
 /// A helper class to cache BackgroundIndexStorage operations and keep the
 /// inverse dependency mapping.
 class BackgroundIndexLoader {
@@ -79,9 +69,11 @@ BackgroundIndexLoader::loadShard(PathRef StartSourceFile, PathRef DependentTU) {
 
   LS.Shard = std::move(Shard);
   for (const auto &It : *LS.Shard->Sources) {
-    auto AbsPath = uriToAbsolutePath(It.getKey(), StartSourceFile);
-    if (!AbsPath)
+    auto AbsPath = URI::resolve(It.getKey(), StartSourceFile);
+    if (!AbsPath) {
+      elog("Failed to resolve URI: {0}", AbsPath.takeError());
       continue;
+    }
     // A shard contains only edges for non main-file sources.
     if (*AbsPath != StartSourceFile) {
       Edges.push_back(*AbsPath);