thakis added inline comments.
================
Comment at: clang-tools-extra/CMakeLists.txt:4
+option(CLANG_TIDY_ENABLE_STATIC_ANALYZER
+ "Include static analyzer checks in clang-tidy" ON)
+
----------------
hans wrote:
> Should this default to CLANG_ENABLE_STATIC_ANALYZER instead of ON?
I had thought about it but decided against it:
- They both default to on so it doesn't matter for the default
- If you turn off CLANG_ENABLE_STATIC_ANALYZER it's not clear you also want to
disable it for clang-tidy
- It mixes cmake options from two different directories which can be a bit hairy
- Having the two toggles act independently is easier to understand
================
Comment at: clang/lib/CMakeLists.txt:24
add_subdirectory(IndexSerialization)
-if(CLANG_ENABLE_STATIC_ANALYZER)
- add_subdirectory(StaticAnalyzer)
----------------
hans wrote:
> Why does removing the condition here work?
As far as I understand, it just impacts which targets CMake generates.
clang/lib/FrontendTool/CMakeLists.txt only adds the dep on
clangStaticAnalyzerFrontend if CLANG_ENABLE_STATIC_ANALYZER is set, so this
doesn't change what gets built for "clang". If you build all targets, this will
now always build the analyzer sources and I suppose it makes it a bit easier to
accidentally depend on clangStaticAnalyzerFrontend , but I don't know of
another way to be able to link this into clang-tidy when it's not built at all
over here.
================
Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn:18
+ } else {
+ values += [ "CLANG_TIDY_ENABLE_STATIC_ANALYZER=" ]
+ }
----------------
hans wrote:
> Why not =0?
Because it's a #cmakedefine01, and the string "0" counts as truthy. See
http://llvm-cs.pcc.me.uk/utils/gn/build/write_cmake_config.py#15
(That should probably print an error if a #cmakedefine gets a "0" argument
since that's basically never what you want.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87118/new/
https://reviews.llvm.org/D87118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
