If you have a concrete suggestion of how to suppress this warning for user-defined operators just in unit tests, great. I don’t think 8 easily-suppressed warnings is an unacceptably large false-positive problem, though. Most warnings have similar problems, and the standard cannot possibly be “must never fire on code where the programmer is actually happy with the behavior”.<br><br>John. <br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 10, 2018 at 17:12 Nico Weber <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">"False positive" means "warning fires but didn't find anything interesting", not "warning fires while being technically correct". So all these instances do count as false positives.<div><br></div><div>clang tries super hard to make sure that every time a warning fires, it is useful for a dev to fix it. If you build with warnings enabled, that should be a rewarding experience. Often, this means dialing back a warning to not warn in cases where it would make sense in theory when in practice the warning doesn't find much compared to the amount of noise it generates. This is why for example clang's -Woverloaded-virtual is usable while gcc's isn't (or wasn't last I checked a while ago) – gcc fires always when it's technically correct to do so, clang only when it actually matters in practice.</div></div><div class="gmail_extra"><br><div class="gmail_quote"></div></div><div class="gmail_extra"><div class="gmail_quote">On Tue, Apr 10, 2018 at 10:21 AM, Roman Lebedev via Phabricator via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br></div></div><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">lebedev.ri added a comment.<br>
<span><br>
In <a href="https://reviews.llvm.org/D44883#1063003" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44883#1063003</a>, @thakis wrote:<br>
<br>
> This landing made our clang trunk bots do an evaluation of this warning :-P It fired 8 times, all false positives, and all from unit tests testing that operator= works for self-assignment. (<a href="https://chromium-review.googlesource.com/c/chromium/src/+/1000856" rel="noreferrer" target="_blank">https://chromium-review.googlesource.com/c/chromium/src/+/1000856</a> has the exact details) It looks like the same issue exists in LLVM itself too, <a href="https://reviews.llvm.org/D45082" rel="noreferrer" target="_blank">https://reviews.llvm.org/D45082</a><br>
<br>
<br>
</span>Right, i guess i only built the chrome binary itself, not the tests, good to know...<br>
<span><br>
> Now tests often need warning suppressions for things like this, and this in itself doesn't seem horrible. However, this change takes a warning that was previously 100% noise-free in practice and makes it pretty noisy – without a big benefit in practice. I get that it's beneficial in theory, but that's true of many warnings.<br>
><br>
> Based on how this warning does in practice, I think it might be better for the static analyzer, which has a lower bar for false positives.<br>
<br>
</span>Noisy in the sense that it correctly diagnoses a self-assignment where one **intended** to have self-assignment.<br>
And unsurprisingly, it happened in the unit-tests, as was expected ^ in previous comments.<br>
**So far** there are no truly false-positives noise (at least no reports of it).<br>
<br>
We could help workaround that the way i initially suggested, by keeping this new part of the diag under it's own sub-flag,<br>
and suggest to disable it for tests. But yes, that<br>
</blockquote></div></div><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 class="m_-5719663234437874043HOEnZb"><div class="m_-5719663234437874043h5"><br>
<br>
Repository:<br>
  rC Clang<br>
<br>
<a href="https://reviews.llvm.org/D44883" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44883</a><br>
<br>
<br>
<br></div></div></blockquote></div></div><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 class="m_-5719663234437874043HOEnZb"><div class="m_-5719663234437874043h5">
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div></div></blockquote></div>