Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Nice, LGTM!
llvm/utils/gn/build/BUILD.gn | ||
---|---|---|
152 ↗ | (On Diff #183860) | I'm surprised there is a config for this, but not for clang-tidy itself. Why is that so? |
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn | ||
19 ↗ | (On Diff #183860) | What should be fixed? I.e. what is the current limitation? |
(Please give me a few more days too. Sorry it's taking so long, have been firefighting all week :-/)
No worries. It is not urgent, I am also a bit swamped over here too. I will get back to this in a couple of days.
Thanks, this is great!
llvm/utils/gn/build/BUILD.gn | ||
---|---|---|
152 ↗ | (On Diff #183860) | 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. |
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn | ||
19 ↗ | (On Diff #183860) | +1. |
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/abseil/BUILD.gn | ||
14 ↗ | (On Diff #183860) | 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) |
llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn | ||
3 ↗ | (On Diff #183860) | 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) |
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn | ||
---|---|---|
19 ↗ | (On Diff #183860) | 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? |
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/abseil/BUILD.gn | ||
14 ↗ | (On Diff #183860) | That script is great! It detected two more checks that have been added recently! :) |
llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn | ||
3 ↗ | (On Diff #183860) | 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. |
lgtm!
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn | ||
---|---|---|
19 ↗ | (On Diff #183860) | Sure, that's fine. (git grep tablegen llvm/utils/gn will show you lots of tablegen examples if you want to tackle these later.) |
llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn | ||
3 ↗ | (On Diff #183860) | Maybe; it's the gn check item in llvm/utils/gn/TODO.txt. |
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn | ||
---|---|---|
19 ↗ | (On Diff #183860) | Sure, I will do it! Thanks! |