[PATCH] D68564: [clangd] Catch an unchecked "Expected<T>" in HeaderSourceSwitch.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373897: [clangd] Catch an unchecked "Expected<T>" in HeaderSourceSwitch. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68564?vs=223464&id=223733#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68564/new/

https://reviews.llvm.org/D68564

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp


Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -460,7 +460,7 @@
   if (auto CorrespondingFile =
           getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem()))
     return CB(std::move(CorrespondingFile));
-  auto Action = [Path, CB = std::move(CB),
+  auto Action = [Path = Path.str(), CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -1045,7 +1045,7 @@
         if (!Path)
           return Reply(Path.takeError());
         if (*Path)
-          Reply(URIForFile::canonicalize(**Path, Params.uri.file()));
+          return Reply(URIForFile::canonicalize(**Path, Params.uri.file()));
         return Reply(llvm::None);
       });
 }
Index: clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp
+++ clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp
@@ -86,7 +86,9 @@
     if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
       if (*TargetPath != OriginalFile) // exclude the original file.
         ++Candidates[*TargetPath];
-    };
+    } else {
+      elog("Failed to resolve URI {0}: {1}", TargetURI, TargetPath.takeError());
+    }
   };
   // If we switch from a header, we are looking for the implementation
   // file, so we use the definition loc; otherwise we look for the header file,
Index: clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -125,6 +125,7 @@
   Testing.HeaderCode = R"cpp(
   void B_Sym1();
   void B_Sym2();
+  void B_Sym3_NoDef();
   )cpp";
   Testing.Filename = "b.cpp";
   Testing.Code = R"cpp(
@@ -163,6 +164,12 @@
          void B_Sym1();
        )cpp",
        testPath("a.cpp")},
+
+       {R"cpp(
+          // We don't have definition in the index, so stay in the header.
+          void B_Sym3_NoDef();
+       )cpp",
+       None},
   };
   for (const auto &Case : TestCases) {
     TestTU TU = TestTU::withCode(Case.HeaderCode);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68564.223733.patch
Type: text/x-patch
Size: 2686 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191008/eea0117d/attachment-0001.bin>