ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Many thanks for improving the running time, this slow test has bugged me since
the day I first ran it.
I'd even be happy to remove it completely. It's slow, possibly flaky, so IMO
the corresponding check does not carry its weight.
I'm not sure if we introduced a new race condition there that will cause flaky
test, see the comment.
Otherwise LGTM.
================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:272
+ [&](std::vector<Diag>) {
+ ADD_FAILURE()
+ << "auto should have been discarded (dead write)";
----------------
This is potentially a race in the test that we did not have before, right?
There is probably much less work in the main test thread, but we probably can
still see the failure if we are unlucky with how the OS schedules us.
My stance is it's better to have no tests for certain behaviours than flaky
tests.
But if you find it useful let's try to run it and see whether my suspicions are
unfounded, don't want to block the change on it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125103/new/
https://reviews.llvm.org/D125103
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
