kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180
+ for (auto &Mod : *Modules)
+ Mod.blockUntilIdle(Deadline::infinity());
+ }
----------------
why is our contract saying that just calling `stop` is not enough?
i think clangdserver should just signal shutdown to modules, and our contract
should say that server facilities will be undefined from this point forward.
that way modules accessing the facilities, could block stop until they are
done, and never make use of it afterwards? it'll make modules a little more
complicated, at the very least they would need some stricter control whenever
they are accessing facilities, but I think it is worth for keeping clangdserver
shutdown cleaner. wdyt?
================
Comment at: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp:261
+ class AsyncCounter : public Module {
+ bool ShouldStop;
+ int State = 0;
----------------
` = false;`
================
Comment at: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp:349
+ ASSERT_TRUE(One && Two && Three) << "sync should wait for pending requests";
+ EXPECT_EQ(3, Counter->getSync()) << "sync avoids flaky tests";
+ // Sanity check: reads and writes are sequenced on the worker thread.
----------------
not sure what this is testing in addition to final callback receiving 3 as a
value.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96755/new/
https://reviews.llvm.org/D96755
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
