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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
