[PATCH] D14192: Add ExtraArgs and ExtraArgsBefore options to enable clang warnings via configuration files.

[Manuel Klimek via cfe-commits]

klimek added inline comments.

================
Comment at: clang-tidy/ClangTidy.cpp:382
@@ +381,3 @@
+      const CommandLineArguments &Args, StringRef Filename) {
+    Context.setCurrentFile(Filename);
+    const ClangTidyOptions &Opts = Context.getOptions();
----------------
alexfh wrote:
> klimek wrote:
> > alexfh wrote:
> > > klimek wrote:
> > > > My concern is still with the Filename in the arg callback. I'd like to understand the full use-case better.
> > > > From this code it looks like we want to change the ClangTidyContext's file and use the callback for that? That seems rather out of place, but maybe I'm missing something.
> > > We need the filename to retrieve the proper ClangTidyOptions for each file. Context.setCurrentFile is used to specify which file should getOptions return the options for.
> > But the extra-args don't depend on the file; it seems like the filename in the callback is only used to be notified when the file changes?
> The thing is that they do depend on the file, because we're implementing their configuration from the config file, and different translation units can have different corresponding configuration files.
Ok, now I get it. I completely didn't expect an interface that needs a setCurrentFile so it can getOptions afterwards for that file.
In that case, the Filename is OK, but I suggest changing the ClangTidyOptions to provide getOptions(file) instead.


http://reviews.llvm.org/D14192