alexfh added a comment.
Nice! I like the idea, but we need to figure out a bunch of details (see the
comments inline).
================
Comment at: clang-tidy/ClangTidyOptions.cpp:264
@@ +263,3 @@
+ ParsedOptions->CheckSources[*ParsedOptions->Checks] =
+ std::string(ConfigFile.c_str());
+ }
----------------
`ConfigFile.str()` should also work.
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:131
@@ -130,1 +130,3 @@
+static cl::opt<bool> ExplainChecks("explain-checks", cl::desc(R"(
+explains where each check comes from, i.e. command line, .clang-tidy
----------------
I would expect something different from an option named '-explain-checks', e.g.
some kind of a short documentation on each check or the like. Something like
`explain-config` or `describe-config` would be closer to what the option does,
I think.
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:132
@@ +131,3 @@
+static cl::opt<bool> ExplainChecks("explain-checks", cl::desc(R"(
+explains where each check comes from, i.e. command line, .clang-tidy
+configuration file.
----------------
How about `for each enabled check explains, where it is enabled, i.e. in
clang-tidy binary, command line or a specific configuration file`?
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:265
@@ -258,2 +264,3 @@
DefaultOptions.Checks = DefaultChecks;
+ DefaultOptions.CheckSources[DefaultChecks] = "ClangTidy binary";
DefaultOptions.WarningsAsErrors = "";
----------------
s/ClangTidy/clang-tidy/
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:278
@@ -270,1 +277,3 @@
OverrideOptions.Checks = Checks;
+ OverrideOptions.CheckSources[Checks] = "command-line argument 'checks'";
+ }
----------------
s/argument 'checks'/option '-checks'/
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:294
@@ +293,3 @@
+ ParsedConfig->CheckSources[*ParsedConfig->Checks] =
+ "command-line argument 'config'";
+ }
----------------
s/argument 'config'/option '-config'/
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:329
@@ +328,3 @@
+ for (const std::string& Check : EnabledChecks) {
+ for (const ClangTidyOptions::StringPair &CheckSource:
+ EffectiveOptions.CheckSources) {
----------------
I'm not sure I understand how this should work in case a check is:
1. enabled in binary (e.g. `-*,a`)
2. disabled in the config file (e.g. `-a`)
3. enabled again on the command line (e.g. `a`)
clang-tidy should say 'command line' in this case, but i'm not sure what your
implementation will do, since it doesn't seem to keep the order of the
configurations.
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:331
@@ +330,3 @@
+ EffectiveOptions.CheckSources) {
+ GlobList filter(CheckSource.first);
+ if (filter.contains(Check)) {
----------------
nit: Filter
You could also write `if (GlobList(CheckSource.first).contains(Check)) {`
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:333
@@ +332,3 @@
+ if (filter.contains(Check)) {
+ llvm::outs() << "'" << Check << "' comes from " << CheckSource.second
+ << ".\n";
----------------
s/comes from/is enabled in the/
Repository:
rL LLVM
http://reviews.llvm.org/D18694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
