[PATCH] Implemented clang-tidy configurability via .clang-tidy files.

[Alexander Kornienko]

================
Comment at: clang-tidy/ClangTidyOptions.cpp:83
@@ +82,3 @@
+
+  if (Other.HeaderFilterRegex)                                                             \
+    Result.HeaderFilterRegex = Other.HeaderFilterRegex;
----------------
djasper wrote:
> I think you want to remove the escaped newline.
Absolutely.

================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:50
@@ +49,3 @@
+                          "set of enabled checks.\n"
+                          "This option's value is appended to the value read\n"
+                          "from a .clang-tidy file, if any."),
----------------
djasper wrote:
> alexfh wrote:
> > djasper wrote:
> > > I am not entirely certain that appending is the right thing to do here. But I understand that otherwise, it would be next to impossible to say "everything in the config file +/- this one check". Then again, with -dump-config, it might be easy enough to copy and past the existing active checks for a given file and modify them.
> > This is done to keep the current behavior. If we want to change it, I'd rather do this as a separate step. And we also need strong reasons to do so, as it would degrade user experience, imo.
> This can't be the current behavior as currently, there are no .clang-tidy files. And that's also why I want us to think about it now, before we have to break existing behavior. In your opinion, what are the pros/cons for/against appending?
The current behavior is that adding -checks=... on the command line modifies the set of checks by appending the new value to the value that would be used without the command line option. The new code behaves exactly in the same way when there's no .clang-tidy file, and naturally extends the same behavior to the case when there is a .clang-tidy file (the "defaults" are taken from the .clang-tidy file instead).

If instead of appending we overwrite the value, we'll change the behavior of clang-tidy in both use cases.

Apart from trying to avoid changes in behavior, I see the following benefit of the current approach: it allows to slightly change the default set of checks as well as completely override it equally easily (use -checks=some-check to add a check to the default set, -checks=-some-check to remove it, -checks=-*,some-check to only run some-check).

================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:91
@@ +90,3 @@
+
+static cl::opt<bool> AnalyzeTemporaryDtors(
+    "analyze-temporary-dtors",
----------------
djasper wrote:
> alexfh wrote:
> > djasper wrote:
> > > Hm. I am wondering whether it really makes sense to keep these individual config flags. The approach in clang-format, where we can just pass in a JSON config through a single flag seems like a good alternative.
> > I'd keep at least the most frequently used options, e.g. -checks:
> > 
> >   clang-tidy -config='{Checks: "..."}'
> > 
> > instead of
> > 
> >   clang-tidy -checks="..."
> > 
> > is not extremely ugly, but a bit less convenient. Anyways, I'd like to make any unnecessary behavior changes in a separate patch.
> Ok, but what are the more frequently used flags? Also, dumping the current config into a .clang-tidy file and changing that seems like the more convenient way to change these things anyway.
> 
> FWIW, I somewhat agree on keeping "-checks" (although I have no data on how frequently it's used). However, I don't thing -analyze-temporary-dtors is or should be a "frequent flag", which is why I left the comment here.
I totally agree with the idea of removing -analyze-temporary-dtors if we add a command-line option to specify JSON config. The -header-filter= option also doesn't look like a frequently used. However, -checks= is used at least by the check developers and in all our documents, so we'd better leave it.

http://reviews.llvm.org/D5186