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

Reply via email to