Author: hokein Date: Mon Oct 7 04:37:25 2019 New Revision: 373897 URL: http://llvm.org/viewvc/llvm-project?rev=373897&view=rev Log: [clangd] Catch an unchecked "Expected<T>" in HeaderSourceSwitch.
Summary: Also fixes a potential user-after-scope issue of "Path". Reviewers: kadircet Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D68564 Modified: 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 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=373897&r1=373896&r2=373897&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Oct 7 04:37:25 2019 @@ -1045,7 +1045,7 @@ void ClangdLSPServer::onSwitchSourceHead 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); }); } Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=373897&r1=373896&r2=373897&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Oct 7 04:37:25 2019 @@ -460,7 +460,7 @@ void ClangdServer::switchSourceHeader( 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()); Modified: clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp?rev=373897&r1=373896&r2=373897&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp (original) +++ clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp Mon Oct 7 04:37:25 2019 @@ -86,7 +86,9 @@ llvm::Optional<Path> getCorrespondingHea 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, Modified: clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp?rev=373897&r1=373896&r2=373897&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp Mon Oct 7 04:37:25 2019 @@ -125,6 +125,7 @@ TEST(HeaderSourceSwitchTest, FromHeaderT 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 @@ TEST(HeaderSourceSwitchTest, FromHeaderT 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
