
<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Marshall,<div class=""><br class=""></div><div class="">I’ve been thinking about this, and I propose we proceed the following way:</div><div class=""><br class=""></div><div class=""><ol class="MailOutline"><li class="">Add a new warning to clang, -Wnonnull-attribute (modulo name bikeshed), which is part of the nonnull group and on by default. That warning specifically controls warnings generated by usage of the nonnull attribute in code.</li><li class="">Wait a week or two so out-of-sync clang / libc++ codebases can catch up.</li><li class="">Un-revert r333351, adding the nonnull attributes to non-member atomic functions.</li></ol><div><br class=""></div><div>That should address the concern you raised because library users can now silence the warning. In this case I think it’s fine to have the warning on by default because:</div><div><br class=""></div><div><ul class="MailOutline"><li class="">It’s hard to trigger, you need to directly pass null to the functions, copying a null into the function won’t warn.</li><li class="">The warning has zero false-positives. The code will definitely segfault when you call it.</li></ul></div><div><br class=""></div><div>What do you think?</div><div><br class=""></div><div><br class=""></div><div>Thanks,</div><div><br class=""></div><div>JF</div><div><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class="">On May 26, 2018, at 12:50 PM, JF Bastien via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="Apple-interchange-newline"><br class=""><blockquote type="cite" class=""><div class="">On May 26, 2018, at 12:36 PM, Marshall Clow <<a href="mailto:mclow.lists@gmail.com" class="">mclow.lists@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Fri, May 25, 2018 at 4:43 PM, JF Bastien via cfe-commits<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Author: jfb<br class="">Date: Fri May 25 16:43:53 2018<br class="">New Revision: 333325<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=333325&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project?rev=333325&view=rev</a><br class="">Log:<br class="">Add nonnull; use it for atomics<br class=""><br class=""></blockquote><div class=""><br class=""></div><div class="">JF - please revert this patch.</div></div></div></div></div></blockquote><div class=""><br class=""></div><div class=""><div class="" style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; background-color: rgb(255, 255, 255);"><span class="" style="font-variant-ligatures: no-common-ligatures;">r333351</span></div></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">Let's have a discussion about how to implement this so that it is more friendly to people with installed code bases.</div><div class="">[ We've had *extremely* loud responses to unilaterally adding warnings - especially ones that can't be easily disabled - to the libc++ code base in the past. ] </div></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Ha! Interesting point which I hadn’t considered. I guess the clang warning can be controlled, but this one cannot? Agreed that’s suboptimal and we should figure out a way to make them controllable.</div><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra">Also, please include both myself and EricWF on all libc++ reviews.</div></div></div></blockquote><br class=""></div><div style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Sorry, will do. I assumed you’d all have Herald auto-add if you wanted to be on a patch :-)</div><div style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class=""></div><br class="" style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">_______________________________________________</span><br style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">cfe-commits mailing list</span><br style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="mailto:cfe-commits@lists.llvm.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">cfe-commits@lists.llvm.org</a><br style="caret-color: rgb(0, 0, 0); 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; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a></div></blockquote></div><br class=""></div></body></html>