[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor
Whisperity via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 24 03:13:08 PDT 2020
whisperity added inline comments.
================
Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:24
+ do \
+ if (!LLVM_WITH_Z3) \
+ return; \
----------------
xazax.hun wrote:
> steakhal wrote:
> > xazax.hun wrote:
> > > I think this might not be the idiomatic way to skip a test. Consider using ` GTEST_SKIP();`.
> > I agree, though that is not yet supported in the `gtest` in the repository.
> > Should we update that to benefit from this macro?
> >
> > There are several places where we could use that like:
> > - [llvm/unittests/ExecutionEngine/MCJIT/MCJITTestAPICommon.h:29](https://github.com/llvm/llvm-project/blob/master/llvm/unittests/ExecutionEngine/MCJIT/MCJITTestAPICommon.h#L29-L33) (expanded 37 times in the codebase)
> > - [llvm/unittests/Support/ThreadPool.cpp:81](https://github.com/llvm/llvm-project/blob/master/llvm/unittests/Support/ThreadPool.cpp#L81-L85) (expanded 7 times in the file)
> > - [llvm/unittests/Support/MemoryTest.cpp:89](https://github.com/llvm/llvm-project/blob/master/llvm/unittests/Support/MemoryTest.cpp#L89-L94) (expanded 10 time in the file)
> I think you should check who updated gtest the last time and ping them what is the process to update it again.
Still maybe this could warrant a `// FIXME: Update GTest and use GTEST_SKIP()` ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78704/new/
https://reviews.llvm.org/D78704
More information about the cfe-commits
mailing list