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

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 5 06:56:32 PST 2019


thakis accepted this revision.
thakis added a comment.

lgtm!



================
Comment at: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn:19
+
+    # TODO(mbonadei): FIXME.
+    # ClangSACheckers
----------------
mbonadei wrote:
> thakis wrote:
> > 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.
> I commented out the two deps below. I will look into it in the next few days (ClangSACheckers uses tablegen and I need to read about it and learn what it does). Is it OK to land this without SACheckers support?
Sure, that's fine.  (`git grep tablegen llvm/utils/gn` will show you lots of tablegen examples if you want to tackle these later.)


================
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 = [
----------------
mbonadei wrote:
> thakis wrote:
> > 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)
> I see that we are not declaring headers to gn. Is this something we'll want to have in the future? I think this is related to how we look at `configs` vs `public_configs` as well.
Maybe; it's the `gn check` item in llvm/utils/gn/TODO.txt.


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

https://reviews.llvm.org/D57329





More information about the llvm-commits mailing list