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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits