[PATCH] D50627: [clangd] Add a testcase for empty preamble.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 16 03:02:35 PDT 2018


ilya-biryukov added a comment.

In https://reviews.llvm.org/D50627#1198595, @hokein wrote:

> I think this patch is in a good scope (for empty preamble). I'd leave it as it is. The failure of GoTodefinition test is caused by an inconsistent behavior of using `lastBuiltPreamble`/`NewPreamble` in TUScheduler, I will send out a separate patch fixing it (based on our discussion).


LG, thanks!

+ a few more nits to the tests



================
Comment at: unittests/clangd/TUSchedulerTests.cpp:333
+                      // We expect to get a non-empty preamble.
+                      ASSERT_TRUE(bool(Preamble));
+                      EXPECT_GT(Preamble->Preamble->Preamble.getBounds().Size,
----------------
NIT: just use `cantFail(std::move(Preamble))` to get the underlying result, no need for an extra `ASSERT_TRUE` here.


================
Comment at: unittests/clangd/TUSchedulerTests.cpp:348
+                      // We expect to get an empty preamble.
+                      ASSERT_TRUE(bool(Preamble));
+                      EXPECT_EQ(Preamble->Preamble->Preamble.getBounds().Size,
----------------
NIT: also use `cantFail`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50627





More information about the cfe-commits mailing list