<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 10, 2013 at 12:29 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">+alexfh who is going to spend time on this</div>
<div><div class="gmail_extra">
<br><div class="gmail_quote">On Fri, Nov 15, 2013 at 7:59 PM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com" target="_blank">ganna@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div>On Nov 15, 2013, at 9:28 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr">On Fri, Nov 15, 2013 at 6:25 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div>On Nov 15, 2013, at 3:24 , Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:</div>
<br><blockquote type="cite"><blockquote class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
While I’m always happy when the analyzer gets more use, the text mode is something we haven’t ever really supported or worked on productizing—it’s always been a lo-fi output format that’s not guaranteed for anything besides debugging. Shipping a tool that reports paths in text mode is a big step, and there could easily be QoI issues we haven’t had to care about until now. (Also, sometimes paths are dozens of notes long.)<br>
</blockquote><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
I'm not too concerned about this - we had Pavel running the static analyzer on all our code, outputting to text format, and analyzing the results, and I don't remember the text format ever being an issue. I don't really have a good idea what other format we'd want to use - our main use case is overlaying the warnings in various steps of our workflow.</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
If we run into problems with the static analyzer's text output, and you say it's not mean to be "production ready", would that mean you wouldn't want to accept patches to make it production ready? :)</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
</div></blockquote><div><br></div></div><div>Patches to make the text output better are certainly fine. We’ve just never put effort into it up until now—for a while the notes weren’t even associated with the right warnings—and we’d need to make sure we don’t optimize the textual notes at the cost of the HTML output. OTOH, the HTML output could also use some love.</div>
<div><br></div><div>Ted, Anna, do you have an opinion here?</div></div></div></blockquote></div></div></div></blockquote><div><br></div></div>Basically, the most comprehensive output you get for the static analyzer is the plist format (used by Xcode), followed by HTML, and finally by text. You don't get the best user experience by looking at text output. We are definitely open to patches that would improve text format, thought it will always be inferior to other formats and visualizations. If consuming plist format is an option in your use case, I'd recommend going for that instead.</div>
</div></blockquote></div></div></div></blockquote><div><br></div><div>It seems that consuming either plist or HTML format is not a good option for us. I'd prefer to implement a custom PathDiagnosticConsumer, so that we have raw data in a form easily usable from the code (unlike plist/HTML in a file). Analyzer currently doesn't provide an interface to pass a custom PathDiagnosticConsumer, but I can implement this, if you have no objections. My idea was to make AnalysisConsumer, that is returned from clang::ento::CreateAnalysisConsumer(...), implement a new public interface, that provides a way to add PathConsumers.</div>
<div><br></div><div>There's one more thing, that is not directly supported by the current diagnostic system in the Static Analyzer: in clang-tidy we'd like to know the name of the checker producing each diagnostic message. PathDiagnostic has BugType and Category fields, which are both arbitrary human-readable strings, but we need to know the exact name of the checker in the form that can be used in the CheckersControlList option to enable/disable the specific checker. I have a sketch of a patch, that does this. The idea of the patch is to add a CheckerName field to the CheckerBase class, and set it in the CheckerManager::registerChecker() method, which will get them from the CheckerRegistry class through register.*Checker() functions. Before I polish the patch and sending for actual review, I wanted to check with you, if there are any concerns regarding this.</div>
<div><br></div><div>-- </div><div>Regards,</div><div>Alexander</div></div>
</div></div>