[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 7 23:20:37 PST 2020


ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:571
+      ADD_FAILURE() << D << Code;
+    assert(AST.getDiagnostics().empty());
 
----------------
`ADD_FAILURE()` should be enough to indicate there are errors to the users.
No need to crash additionally. 

Could you remove this assert?


================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:750
             void foo(int a, int b) {
-              $0^FOO+$1^BAR;
+              (void)($0^FOO+$1^BAR);
             }
----------------
Could we assert there are no errors instead?
Having warnings is totally fine and allows to avoid changing testcases (like this one)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72355/new/

https://reviews.llvm.org/D72355





More information about the cfe-commits mailing list