[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