<div dir="ltr">> There's a fairly substantial difference between warnings that target patterns<br>
> that are <em>inarguably</em> bad, like the std::move problem (which in some sense is<br>
> a language defect that people just need to be taught how to work around), and<br>
> warnings that target code patterns that might be confusing or which have a<br>
> higher-than-normal chance of being a typo.  <code style="background-color:rgb(247,247,247);border-radius:3px;margin:0px;padding:0px 0.4em" bgcolor="#F7F7F7">-Wparentheses</code> is the classic<br><div>
> example here</div><div><br></div><div>Yes, and this warning is definitely like -Wparentheses: something that could be wrong, but is not necessarily.</div><div>Still, I think it will catch tricky bugs.<br></div><div><br></div><div>What should we do about this warning/patch?</div><div>Deactivate it by default, with its own flag? without putting it in Wall/extra?</div><div>Or wait for something like your proposal to be implemented?</div><div><div>Or just forget about it for now and simply wait for more feedback from the mailing before deciding?</div></div><div><br></div><div>Arnaud<br></div><div><br></div><div><br></div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">Le mer. 9 janv. 2019 à 01:14, John McCall <<a href="mailto:jmccall@apple.com">jmccall@apple.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>




<div>
<div style="font-family:sans-serif"><div style="white-space:normal">
<p dir="auto">On 8 Jan 2019, at 15:03, Arthur O'Dwyer wrote:</p>

</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid rgb(119,119,119);color:rgb(119,119,119);margin:0px 0px 5px;padding-left:5px"><p dir="auto">I was thinking of `-Wreturn-std-move`, which is -Wmove/-Wmost/-Wall but not<br>
always-on.</p>
</blockquote></div>
<div style="white-space:normal">

<p dir="auto">I honestly don't know why this isn't default-on.</p>

</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid rgb(119,119,119);color:rgb(119,119,119);margin:0px 0px 5px;padding-left:5px"><p dir="auto">Grepping the code for DefaultIgnore, I see that -Wfor-loop-analysis is<br>
another example (but 4 years old).</p>
</blockquote></div>
<div style="white-space:normal">

<p dir="auto">This is a harder call.  At any rate, I think my point stands that this is not<br>
"frequent".</p>

<p dir="auto">There's a fairly substantial difference between warnings that target patterns<br>
that are <em>inarguably</em> bad, like the std::move problem (which in some sense is<br>
a language defect that people just need to be taught how to work around), and<br>
warnings that target code patterns that might be confusing or which have a<br>
higher-than-normal chance of being a typo.  <code style="background-color:rgb(247,247,247);border-radius:3px;margin:0px;padding:0px 0.4em" bgcolor="#F7F7F7">-Wparentheses</code> is the classic<br>
example here: it unquestionably catches a common mistake that C unfortunately<br>
otherwise masks, but it's still perennially controversial because the<br>
assign-and-test idiom is so common in C programming, and there are a lot of<br>
people who still swear by reversing the equality test (<code style="background-color:rgb(247,247,247);border-radius:3px;margin:0px;padding:0px 0.4em" bgcolor="#F7F7F7">0 == foo</code>) instead of<br>
relying on the warning.</p>

<p dir="auto">John.</p>

</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid rgb(119,119,119);color:rgb(119,119,119);margin:0px 0px 5px;padding-left:5px"><p dir="auto">On Tue, Jan 8, 2019 at 2:37 PM John McCall <<a href="mailto:jmccall@apple.com" target="_blank">jmccall@apple.com</a>> wrote:<br>
</p>
<blockquote style="border-left:2px solid rgb(153,153,153);color:rgb(153,153,153);margin:0px 0px 5px;padding-left:5px"><p dir="auto">On 8 Jan 2019, at 13:45, Arthur O'Dwyer wrote:<br>
</p>
<blockquote style="border-left:2px solid rgb(187,187,187);color:rgb(187,187,187);margin:0px 0px 5px;padding-left:5px"><p dir="auto">On Tue, Jan 8, 2019 at 1:05 PM John McCall via cfe-dev <<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
</p>
<blockquote style="border-left:2px solid rgb(187,187,187);color:rgb(187,187,187);margin:0px 0px 5px;padding-left:5px"><p dir="auto">On 8 Jan 2019, at 8:37, Arnaud Bienner wrote:</p>
<blockquote style="border-left:2px solid rgb(187,187,187);color:rgb(187,187,187);margin:0px 0px 5px;padding-left:5px"><p dir="auto">I realized I didn't put "DefaultIgnore" on this warning.<br>
I'm not experienced enough with clang to know what's the best way to<br>
deal<br>
with new warnings, but my feeling is that it would be sensible to<br>
have</p>
</blockquote><p dir="auto">this</p>
<blockquote style="border-left:2px solid rgb(187,187,187);color:rgb(187,187,187);margin:0px 0px 5px;padding-left:5px"><p dir="auto">new warning DefaultIgnore for now, in -Wall, and make it default<br>
once we<br>
have some feedback from the community: while not all C++ projects<br>
use</p>
</blockquote><p dir="auto">-Wall</p>
<blockquote style="border-left:2px solid rgb(187,187,187);color:rgb(187,187,187);margin:0px 0px 5px;padding-left:5px"><p dir="auto">(or -Wextra) I believe enough do to give us a chance to get some</p>
</blockquote><p dir="auto">feedback.</p>
<blockquote style="border-left:2px solid rgb(187,187,187);color:rgb(187,187,187);margin:0px 0px 5px;padding-left:5px"><p dir="auto">What do you think?</p>
</blockquote><p dir="auto">We generally don't add things to -Wall.  That's why I went into my<br>
whole<br>
spiel<br>
about versioning: I think it's a conversation we need to have before<br>
we're<br>
ready to accept this as a warning that's anything but hidden<br>
permanently<br>
behind its own opt-in flag.<br>
</p>
</blockquote><p dir="auto">John: Wha? Clang *frequently* adds things to -Wall!  -Wall includes<br>
-Wmost<br>
which includes a bunch of other categories, so while we don't often<br>
put new<br>
diagnostics *directly* under -Wall, pretty much every "reasonable"<br>
diagnostic eventually winds up in there somehow — which is the<br>
intent.</p>
</blockquote><p dir="auto">I don't think we "frequently" add things to -Wall or -Wmost.  We do<br>
somewhat<br>
frequently add warnings that are unconditionally default-on, but the<br>
groups<br>
have a conventional meaning that we don't generally touch.  What<br>
recently-added<br>
warnings are you thinking of that are not default-on but which are<br>
included<br>
in a group like -Wall or -Wmost?<br>
<br>
John.<br>
</p>
</blockquote></blockquote></div>
<div style="white-space:normal">
</div>
</div>
</div>

</blockquote></div>