From 27b83d230d182d2fab0e132f7f75f203cf71d95a Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Thu, 22 Nov 2018 16:20:12 +0000 Subject: [PATCH] [clangd] Cleanup error consumption code. NFC - Remove reimplementations of llvm::consumeError. - Simplify test code by using EXPECT_ERROR where it fits. llvm-svn: 347472 --- clang-tools-extra/clangd/ClangdServer.cpp | 6 +--- clang-tools-extra/unittests/clangd/Matchers.h | 4 +-- .../unittests/clangd/TUSchedulerTests.cpp | 26 ++++++----------- .../unittests/clangd/URITests.cpp | 29 +++++-------------- 4 files changed, 18 insertions(+), 47 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 03ebcc60a279..27c9089e2f3c 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -39,10 +39,6 @@ namespace clang { namespace clangd { namespace { -void ignoreError(Error Err) { - handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {}); -} - std::string getStandardResourceDir() { static int Dummy; // Just an address in this process. return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy); @@ -312,7 +308,7 @@ void ClangdServer::dumpAST(PathRef File, unique_function Callback) { auto Action = [](decltype(Callback) Callback, Expected InpAST) { if (!InpAST) { - ignoreError(InpAST.takeError()); + llvm::consumeError(InpAST.takeError()); return Callback(""); } std::string Result; diff --git a/clang-tools-extra/unittests/clangd/Matchers.h b/clang-tools-extra/unittests/clangd/Matchers.h index ed44464d0c14..d666de36900f 100644 --- a/clang-tools-extra/unittests/clangd/Matchers.h +++ b/clang-tools-extra/unittests/clangd/Matchers.h @@ -125,9 +125,7 @@ PolySubsequenceMatcher HasSubsequence(Args &&... M) { << ::testing::PrintToString(*ComputedValue); \ break; \ } \ - handleAllErrors(ComputedValue.takeError(), \ - [](const llvm::ErrorInfoBase &) {}); \ - \ + llvm::consumeError(ComputedValue.takeError()); \ } while (false) } // namespace clangd diff --git a/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp b/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp index 8f5b4c6f4033..e5f29141ebab 100644 --- a/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp +++ b/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "Context.h" +#include "Matchers.h" #include "TUScheduler.h" #include "TestFS.h" #include "gmock/gmock.h" @@ -29,9 +30,6 @@ using ::testing::Pointee; using ::testing::UnorderedElementsAre; void ignoreUpdate(Optional>) {} -void ignoreError(Error Err) { - handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {}); -} class TUSchedulerTests : public ::testing::Test { protected: @@ -61,15 +59,11 @@ TEST_F(TUSchedulerTests, MissingFiles) { // Assert each operation for missing file is an error (even if it's available // in VFS). - S.runWithAST("", Missing, [&](Expected AST) { - ASSERT_FALSE(bool(AST)); - ignoreError(AST.takeError()); - }); - S.runWithPreamble("", Missing, TUScheduler::Stale, - [&](Expected Preamble) { - ASSERT_FALSE(bool(Preamble)); - ignoreError(Preamble.takeError()); - }); + S.runWithAST("", Missing, + [&](Expected AST) { EXPECT_ERROR(AST); }); + S.runWithPreamble( + "", Missing, TUScheduler::Stale, + [&](Expected Preamble) { EXPECT_ERROR(Preamble); }); // remove() shouldn't crash on missing files. S.remove(Missing); @@ -83,14 +77,12 @@ TEST_F(TUSchedulerTests, MissingFiles) { S.remove(Added); // Assert that all operations fail after removing the file. - S.runWithAST("", Added, [&](Expected AST) { - ASSERT_FALSE(bool(AST)); - ignoreError(AST.takeError()); - }); + S.runWithAST("", Added, + [&](Expected AST) { EXPECT_ERROR(AST); }); S.runWithPreamble("", Added, TUScheduler::Stale, [&](Expected Preamble) { ASSERT_FALSE(bool(Preamble)); - ignoreError(Preamble.takeError()); + llvm::consumeError(Preamble.takeError()); }); // remove() shouldn't crash on missing files. S.remove(Added); diff --git a/clang-tools-extra/unittests/clangd/URITests.cpp b/clang-tools-extra/unittests/clangd/URITests.cpp index b094c8914ceb..97fa59eabc32 100644 --- a/clang-tools-extra/unittests/clangd/URITests.cpp +++ b/clang-tools-extra/unittests/clangd/URITests.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "Matchers.h" #include "TestFS.h" #include "URI.h" #include "gmock/gmock.h" @@ -77,16 +78,9 @@ TEST(URITest, Create) { } TEST(URITest, FailedCreate) { - auto Fail = [](Expected U) { - if (!U) { - consumeError(U.takeError()); - return true; - } - return false; - }; - EXPECT_TRUE(Fail(URI::create("/x/y/z", "no"))); + EXPECT_ERROR(URI::create("/x/y/z", "no")); // Path has to be absolute. - EXPECT_TRUE(Fail(URI::create("x/y/z", "file"))); + EXPECT_ERROR(URI::create("x/y/z", "file")); } TEST(URITest, Parse) { @@ -120,21 +114,12 @@ TEST(URITest, Parse) { } TEST(URITest, ParseFailed) { - auto FailedParse = [](StringRef U) { - auto URI = URI::parse(U); - if (!URI) { - consumeError(URI.takeError()); - return true; - } - return false; - }; - // Expect ':' in URI. - EXPECT_TRUE(FailedParse("file//x/y/z")); + EXPECT_ERROR(URI::parse("file//x/y/z")); // Empty. - EXPECT_TRUE(FailedParse("")); - EXPECT_TRUE(FailedParse(":/a/b/c")); - EXPECT_TRUE(FailedParse("\"/a/b/c\" IWYU pragma: abc")); + EXPECT_ERROR(URI::parse("")); + EXPECT_ERROR(URI::parse(":/a/b/c")); + EXPECT_ERROR(URI::parse("\"/a/b/c\" IWYU pragma: abc")); } TEST(URITest, Resolve) {