[PATCH] D143804: [bazel] create a clang-tidy binary target

[Aaron Siddhartha Mondal via Phabricator via llvm-commits]

aaronmondal added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel:19
+# Include static analyzer checks in clang-tidy. Usage:
+#   $ bazel build -- at llvm-project//clang-tools-extra/clang-tidy:enable_static_analyzer=true //...
+#   $ bazel build -- at llvm-project//clang-tools-extra/clang-tidy:enable_static_analyzer=false //...
----------------
GMNGeoffrey wrote:
> Note that we can also add a flag alias in the project bazelrc so that in-repo builds have a short way to spell this.
Yes, although I think we should try to not depend on the `.bazelrc` too much. If someone imports the project, the bazelrc is ignored (btw this also can cause builds to break already since we set the C++ standard in the .bazelrc and a naive workspace or module will try to build LLVM with an older, incompatible C++ standard).

We probably need "standardized" way to handle these config flags in general. If we move the entire LLVM config to build settings we'll have to handle like 50 config options. Improving the platforms may reduce that complexity a bit, but we'd probably still end up with quite a few manually tunable knobs.


================
Comment at: utils/bazel/llvm-project-overlay/clang-tools-extra/clang-tidy/BUILD.bazel:67
+    hdrs = [":confusables_inc"],
+    include_prefix = ".",
+)
----------------
GMNGeoffrey wrote:
> I think this unnecessary, right? That's the default. The headers should already be available at this path
I believe this case is different since we are handling a generated header. I remember that Bazel would raise an error if this was pointing to the workspace root, but I'm not sure whether the behavior has changed (or whether it is different for a non-root package).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143804