ilya-biryukov added a comment.
Thanks, we were previously getting inconsistent ASTs (i.e. using different
preambles), depending on whether they were built in `TUScheduler::update` or in
`TUScheduler::runWithAST` handlers.
Just a few NITs.
================
Comment at: clangd/TUScheduler.cpp:373
std::lock_guard<std::mutex> Lock(Mutex);
- if (NewPreamble)
- LastBuiltPreamble = NewPreamble;
+ // Always use the NewPreamble.
+ LastBuiltPreamble = NewPreamble;
----------------
Maybe remove the comment? It does not seem to add any additional information on
top of the code.
================
Comment at: unittests/clangd/XRefsTests.cpp:1058
+ // stale one.
+ Server.findDefinitions(
+ FooCpp, FooWithoutHeader.point(),
----------------
Maybe use `runFindDefinitions` instead of the two `Server.findDefinitions()` +
`Server.blockUntilIdleForTest()` calls?
================
Comment at: unittests/clangd/XRefsTests.cpp:1061
+ [&](llvm::Expected<std::vector<Location>> Loc) {
+ ASSERT_TRUE(bool(Loc));
+ EXPECT_THAT(*Loc,
----------------
NIT: just use `cantFail(std::move(Loc))` to get the underlying vector and
remove `ASSERT_TRUE`.
This gives equivalent results (crashes tests with `assert` failure internally
in LLVM) with a simpler syntax.
================
Comment at: unittests/clangd/XRefsTests.cpp:1073
+ // Use the AST being built in above request.
+ Server.findDefinitions(
+ FooCpp, FooWithoutHeader.point(),
----------------
Same here: maybe use `runFindDefinitions`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50695
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
