[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