[PATCH] D57329: [gn] Checking in BUILD.gn files for clang-tidy and clang-apply-replacements

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Thanks, this is great!



================
Comment at: llvm/utils/gn/build/BUILD.gn:152
 
+config("clang_apply_replacements_code") {
+  include_dirs = [
----------------
yvesg wrote:
> I'm surprised there is a config for this, but not for clang-tidy itself. Why is that so?
I think we should remove this and instead put `  include_dirs = [ "include" ]` and `  include_dirs = [ "../include" ]` respectively in the two clang-apply-replacement build files instead. I think the clang-extra-tools stuff is intended to look like tools in llvm/tools or clang/tools and not like a full lib, and clang-apply-replacements is just weird in doing this full include and lib subtree thing for what is effectively 3 files.


================
Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn:19
+
+    # TODO(mbonadei): FIXME.
+    # ClangSACheckers
----------------
yvesg wrote:
> What should be fixed? I.e. what is the current limitation?
> Static Analyzer checks not available? If so, the following deps could be commented.
+1.


================
Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/abseil/BUILD.gn:14
+  sources = [
+    "AbseilTidyModule.cpp",
+    "DurationComparisonCheck.cpp",
----------------
Because it took me so long, this is now missing

    "DurationAdditionCheck.cpp",

(courtesy of `llvm/utils/gn/build/sync_source_lists_from_cmake.py` after rebasing locally)


================
Comment at: llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn:3
   output_name = "clangBasic"
-  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  public_configs = [ "//llvm/utils/gn/build:clang_code" ]
   public_deps = [
----------------
Instead of this, I think it'd be better to say `configs += [ "//llvm/utils/gn/build:clang_code" ]` in all the clang-tools-extra targets (like all the clang targets do)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57329