<div dir="ltr">On Mon, Nov 18, 2013 at 5:47 PM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div class="im"><div><br></div><div><div>On Nov 18, 2013, at 0:17 , Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr">On Sun, Nov 17, 2013 at 11:48 AM, Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> wrote:<br><div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>It seems weird to be using FrontendAction for clang-tidy (and the analyzer too). clang-tidy isn't a compiler frontend (I mean look at the methods: hasCodeCompletionSupport? getCurrentFileKind? shouldEraseOutputFiles?). AFAICT All you are really need is a vector of PPCallbacks, and a vector of what is effectively std::function<TidyResult(ASTContext &)>. It seems like there should be at most a single FrontendAction which just sets things up so that it can forward everything down into those vectors.</div>

</div></blockquote><div><br></div><div>That is basically what I proposed as "b". The problem with that is that this is not how the static analyzer is currently factored - if I want to pull it apart into a PPCallbacks and an ASTConsumer, I have to duplicate significant amounts of code. Thus, if we want to go that way, the next step is to make the static analyzer more modular: basically pull out a class that can give you both the PPCallbacks and the ASTConsumer. But on the other hand, such a class already exists - it is the static analyzer's FrontendAction; it is exactly what we want, namely a "factory" class (well, in the broader sense) of a PPCallbacks and ASTConsumer instance, which happens to also be able to put the compiler into the state it needs it in.</div>

<div><br></div><div>Perhaps when you argue on the interface level, you mean "there should really be something in between a FrontendAction and an ASTConsumer or PPCallbacks interface", and perhaps you're right, but that sounds like a much bigger change...</div>

<div><br></div><div>Cheers,</div><div>/Manuel</div></div></div></div></blockquote><br></div></div><div>It’s been a while since I’ve looked at the frontend-ish parts of the analyzer, but AFAICT it’s not actually using PPCallbacks in any way. I don’t think there’s any problem with jumping directly to the AnalysisConsumer.</div>
<div><br></div><div>That said, if we ever want to add preprocessing-time checks, they wouldn’t be included in clang-tidy without a separate mechanism, but I think I’m okay with that. The current model is certainly intended to behave as an idempotent ASTConsumer.</div>
</div></blockquote><div><br></div><div>Ah, cool - then I'll try that and see how it goes... Thanks!</div></div></div></div>