sammccall accepted this revision.
sammccall added a comment.
Awesome, thank you.
Just a couple of last nits.
================
Comment at: unittests/clangd/ClangdTests.cpp:984
+ std::vector<Diag> Diagnostics) override {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ for(const Diag& D : Diagnostics) {
----------------
Locking is unneccesary. (Diagnostics will be delivered once, and the SyncAPI
calls block until diagnostics are delivered)
================
Comment at: unittests/clangd/ClangdTests.cpp:1014
+ runAddDocument(Server, SourcePath, Test.code());
+ EXPECT_TRUE(DiagConsumer.workedAsExpected());
+ DiagConsumer.reset();
----------------
nit: the logic is right, but the message could be better.
If we change something and it fails it will print `Expected true:
DiagConsumer.worksAsExpected(), but was false` or so. There are a number of
things that could be wrong.
Prefer just to capture a bit more data (all diagnostics) and use a matcher
expression:
```
MATCHER(DeprecationWarning, "") {
return arg.Category == "Deprecations" && arg.Severity ==
DiagnosticsEngine::Warning;
}
...
EXPECT_THAT(Diags, ElementsAre(DeprecationWarning()));
```
That way if there isn't exactly one diagnostic that's a deprecation warning,
it'll print the expectation, the full list of diagnostics, and the reason it
didn't match.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51747
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
