<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 29, 2016, at 10:38 AM, Reid Kleckner <<a href="mailto:rnk@google.com" class="">rnk@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 29, 2016 at 10:29 AM, Chris Bieneman via llvm-dev <span dir="ltr" class=""><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><div class="">I haven’t done a full audit, but we have 257 XFAILs in LLVM.</div><div class=""><br class=""></div><div class="">* 44 of those are vg_leak failures on TableGen tests which should be UNSUPPORTED because we allow TableGen to leak for performance the same way we allow clang to leak.</div><div class="">* 15 of them are vg_leak in the OCaml bindings. Someone familiar with OCaml should chime in on it, but I suspect those too should be UNSUPPORTED</div></div></div></blockquote><div class=""><br class=""></div><div class="">Do we still support running the test suite with Valgrind? I don't think we run it continuously anymore. Maybe these are just noise now and we should remove them. Users can still run Valgrind on LLVM, but LLVM's test suite is an implementation detail, and we don't have to gaurantee that it is leak free.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><div class="">* 125 of them are universal failure (XFAIL: *). Many of these have been marked this way for years. I suspect that if we take the time to go through these we will likely find that many of these test cases either should be tracked by bugs, or should be removed from the tree</div><div class=""><br class=""></div><div class="">From there, many of the test cases are XFAIL on features where they really should be UNSUPPORTED. I suspect that if we do a full audit of the XFAILs we would likely find <100 which should actually be XFAIL, and tracking those seems valuable to me.</div></div></div></blockquote><div class=""><br class=""></div><div class="">Bummer. <br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><span class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><div class="gmail_quote"><div class="">Auditing existing XFAILs can be done today without filing bugs for them.<br class=""></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">Yes, we can audit them today. Making bugs required for them makes it easier to audit them because there will (in theory) be a bug describing the justification for the XFAIL and what the underlying issue is. Digging up the reason why an XFAIL was added in 2009 is a little bit challenging today if there isn’t a PR associated with it (or a really good comment or commit message).</div></div></div></blockquote><div class=""><br class=""></div><div class="">Can we require a comment for every XFAIL? That seems like a clear guideline that can be enforced by code review, and it's less heavyweight.</div></div></div></div>
</div></blockquote></div><br class=""><div class="">Requiring comments for XFAIL is much harder to enforce via tools, and would need to be enforced with code review. One advantage I see to this process is that the tooling support is a relatively simple patch to LIT.</div><div class=""><br class=""></div><div class="">-Chris</div></body></html>