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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
