<div dir="ltr">Ping. We have had some discussions about this at the EuroLLVM conference, but it would be great to have this on cfe-dev as well.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 26 Mar 2019 at 19:11, Kristóf Umann <<a href="mailto:dkszelethus@gmail.com">dkszelethus@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div dir="ltr">Just a polite ping, Devin, could you please weight in on the responses?</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 20 Mar 2019 at 12:23, Kristóf Umann <<a href="mailto:dkszelethus@gmail.com" rel="noreferrer" target="_blank">dkszelethus@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>Hi Devin!<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Gábor Horváth <<a href="mailto:xazax.hun@gmail.com" rel="noreferrer" target="_blank">xazax.hun@gmail.com</a>> ezt írta (időpont: 2019. márc. 18., H, 11:08):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hi Devin!<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 18 Mar 2019 at 05:04, Devin Coughlin <<a href="mailto:dcoughlin@apple.com" rel="noreferrer" target="_blank">dcoughlin@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>Hi Kristóf,<div><br></div><div>It’s great to see this area getting cleaned up!<br><div><div><br><blockquote type="cite"><div>On Mar 16, 2019, at 1:32 PM, Kristóf Umann <<a href="mailto:dkszelethus@gmail.com" rel="noreferrer" target="_blank">dkszelethus@gmail.com</a>> wrote:</div><br class="gmail-m_7055783802759136616m_-1786147106409467422gmail-m_5351338434827872844gmail-m_3394848771457347449gmail-m_4037830019495472199gmail-m_1048509135953524897Apple-interchange-newline"><div><div dir="ltr"><div dir="ltr"><div>Hi!<br></div><div><br>TL;DR: The API for registering checker options changes once several changes that are up for review now land. This affects out-of-tree and checker plugin developers. Now's the time to participate in the discussion!<br></div></div></div></div></blockquote><div><br></div><div>As a reminder, LLVM encourages an incremental development process where for significant changes it is important to discuss the change and gather consensus. This can avoiding wasted effort implement an approach that doesn’t have consensus. See <<a href="https://www.llvm.org/docs/DeveloperPolicy.html#incremental-changes" rel="noreferrer" target="_blank">https://www.llvm.org/docs/DeveloperPolicy.html#incremental-changes</a>>. It also makes the patches easier to review.</div></div></div></div></div></blockquote></div></div></blockquote><div><br></div><div>I realized only later that I've forgotten to add you as reviewer to my patches. Sorry about that! I probably should've put more effort into reaching out to a wider audience, although discussions in between Artem, George, Gábor and me have been going on for months here on cfe-dev and phabricator. I did not, however, emphasise enough the improtance of some of these patches. For the sake of completeness, I've included a couple links to the earlier discussions we had:<br><br><a href="http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-October/059664.html</a> (start of a thread on changes to AnalyzerOptions)</div><div><a href="http://lists.llvm.org/pipermail/cfe-dev/2018-October/059962.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-October/059962.html</a> (discussion on whether, and if so, how plugins should be supported regarding checker options) <br></div><div><a href="http://lists.llvm.org/pipermail/cfe-dev/2018-December/060519.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-December/060519.html</a> (notice on landing API changes to non-checker config options and checker dependency handling)</div><div><a href="https://reviews.llvm.org/D54438" rel="noreferrer" target="_blank">https://reviews.llvm.org/D54438</a> (checker dependency patch with * a lot * discussion oh things should go, also discussing in detail the issue of plugins)</div><div> <br></div><div>Going forward, I'll be sure to add you as reviewer on my patches, and send out [RFC] tagged mails before working on API touching changes.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div><div><div><br></div><blockquote type="cite"><div><div dir="ltr"><div dir="ltr"><div>In (not overwhelmingly) more detail:<br><br>After many-many months of hard work, I'm very confident in the current state of the project. Allow me to elaborate.</div><div><br></div><div>In the recent months, the frontend of the analyzer, specifically how command line options are handled, have changed a lot. Right now, compared to how things used to be,</div><div><br></div><div>* We can list non-checker analyzer configurations</div><div>* We can verify user input for non-checker analyzer configurations</div><div>* The interface of AnalyzerOptions changed dramatically in order not to allow this problem to arise again</div><div>* debug.ConfigDumper contains all non-checker analyzer configurations, making it an actually usable debug tool</div><div> * An almost decade-old issue, the checker naming bug was resolved by reimplementing checker dependencies, and the related interface was also changed to guard against this happening again</div><div><br></div><div>You probably noticed that I put a very strong emphasis on <i>non-checker analyzer configurations</i> -- since checkers can be loaded run-time via plugins, doing the same for them is a far more difficult task.</div></div></div></div></blockquote><div><br></div><div>I’m not sure how much emphasis we should put on checkers loaded via plugins. The analyzer really doesn’t support a plugin model and probably never will, given the difficulty of maintaining a stable C++ ABI when the components we depend on (such as the AST) don’t expose one in C++.</div></div></div></div></div></blockquote><div><br></div><div>I am kind of surprised that C++ ABI is a concern. While I do admint that it is kind of a fragile solution LLVM does support loading passes dynamically that are using the C++ API. What is the difference between LLVM and CSA in this regard?<br></div></div></div></blockquote><div><br></div><div>We use the plugin feature extensively in Ericsson to register internal checkers that are verifying internal product specific design rules which are of no interest of the community. They are maintained by developers of the analyzer who are most of the time very aware of the changes in between versions, so it's of utmost importance to us that the changes related to dependency and command line option handling is working with plugins as well.</div><div><br></div><div>Would you be more comfortable with these changes if they didn't touch the examples/ folder? Since some extra code is required for plugins to work, I would argue on the side of talking about them in *developer* docs, even if we add scary notes around it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div><div><br><blockquote type="cite"><div><div dir="ltr"><div dir="ltr"><div>I've uploaded several patches that finally fixes this for good. Although these have up for more than a month now, the code changed quite a bit, and after several in-office discussions, vigorous testing and refactoring, I'm very confident that everything is in it's final place. Please take a look if these changes affect you!</div><div><br></div><div>The most important of these patches is <a href="https://reviews.llvm.org/D57855" rel="noreferrer" target="_blank">https://reviews.llvm.org/D57855</a>. Please visit the "Stack" as well, the list of patches that depend on this and those this depends on. While 11 patches might seem a little scary at first, I've put a lot of effort into making as small as possible, in order to easy on reviewing. Note that the one patch I highlighted here is quite large however.</div></div></div></div></blockquote><div><br></div><div>I commented on a bunch of these.</div></div></div></div></div></blockquote></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div><div><div><br></div><div>It is really, really great to see the improvements in testability and specification here. As I noted in some of the patches, I do have some serious concerns about the changes to the user model and command-line flags — but I don’t want those to get in the way of the general goodness of many of the improvements here.</div></div></div></div></div></blockquote><div><br></div><div>Clang does have a history of having flags that are exclusively for developers -Weverything is being one example. I think if those flags make developing clang more convenient it might be worth to have them while making it clear in the documentation and the output that this is not intended to be used by end user. What do you think, would such notices help?</div></div></div></blockquote><div><br></div><div>Shooting in the dark here, so please feel free to correct me, but isn't everything behind -cc1 a "power user only" thing? Since we already list debug and alpha checkers for the flag -analyzer-checker-help (and recently, -analyzer-config-help),</div><div><br></div><div>I have had thoughts about exposing some of these flags however through driver flags, and narrowing these options down to the few that aren't developer-only features (like expand-macros, or  alpha.cplusplus.UninitializedObject:Pedantic). Also, explicitly noting each flag whether they are developer-only or not sounds great too.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>Regards,</div><div>Gábor<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div><div><div>It would probably be a good idea to tease apart the patches that change the user model from those that improve analyzer infrastructure. Let's get the infrastructure ones landed!<br></div></div></div></div></div></blockquote></div></div></blockquote><div><br></div><div>I'm a little unsure about this one, could you specify? I would believe that the patches are already structured that way.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div>Devin</div></div></div></blockquote></div></div></blockquote><div><br></div><div>Thank you so much for the encouraging words on the effort, as well as the feedback on my patches! I'm very excited to see this project finally landing. </div><div><br></div><div>Cheers,</div><div>Kristóf </div></div></div></div>
</blockquote></div>
</blockquote></div>