<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 31, 2015 at 10:06 AM, Richard <span dir="ltr"><<a href="mailto:legalize@xmission.com" target="_blank">legalize@xmission.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
In article <<a href="mailto:CAENS6EsAsF0NYiZkb2x_cbLm2HWH_m73Jd5rkAGGNkZnCLv1tw@mail.gmail.com">CAENS6EsAsF0NYiZkb2x_cbLm2HWH_m73Jd5rkAGGNkZnCLv1tw@mail.gmail.com</a>>,<br>
<span class=""> David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> writes:<br>
<br>
> On Wed, Mar 25, 2015 at 3:24 PM, Richard <<a href="mailto:legalize@xmission.com">legalize@xmission.com</a>> wrote:<br>
<br>
</span><span class="">> > AFAIK, all the smart pointer classes have bool conversion operators.<br>
> ><br>
><br>
> They do, but they're explicit and my point was "return bool(sp);" is less<br>
> good than "return sp != nullptr;" - so without knowing the domain-specific<br>
> bool test it's probably hard to pick/suggest the best fix.<br>
<br>
</span>Exactly.<br>
<br>
It is hard to know the best idiom without coding in a giant table that<br>
has to be updated all the time.<br>
<br>
In the case of a return statement, you don't need to do anything.<br>
<br>
If they wrote 'if (sp != nullptr) return true; else return false;',<br>
then the tool would replace this with 'return sp != nullptr;'.<br>
<br>
If they wrote 'if (sp) return true; else return false;', the tool<br>
would replace this with 'return sp;'.<br></blockquote><div><br>This ^ transformation is most likely not valid for any modern user defined type (because they'll have an explicit operator bool, not an implicit one):<br><br><div>$ cat sp.cpp</div><div>#include <memory></div><div><br></div><div>bool f1(std::unique_ptr<int> u) {</div><div> if (u)</div><div> return true;</div><div> return false;</div><div>}</div><div><br></div><div>bool f2(std::unique_ptr<int> u) {</div><div> return u;</div><div>}</div><div>$ clang++-tot sp.cpp -fsyntax-only -std=c++11</div><div>sp.cpp:10:10: error: no viable conversion from 'std::unique_ptr<int>' to 'bool'</div><div> return u;</div><div> ^</div><div>1 error generated.</div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
In both cases, the tool assumes that you wrote one or the other based<br>
on your preferences of what is readable and it shouldn't be changed.<br>
<br>
It's only in the case of a ternary operator embedded within a larger<br>
expression that you have to do something. I don't want the tool to subtly<br>
change the type of the expression from bool to something else that is<br>
implicitly convertible to bool but might be interpreted as something<br>
other than bool in the larger context.<br>
<br>
However, in the case of a ternary operation, something like:<br>
<br>
foo ^= (p ? true : false);<br>
<br>
changing that to:<br>
<br>
foo ^= p;<br>
<br>
can introduce errors depending on the type of foo and p. In these<br>
cases, at the very least the tool should do:<br>
<br>
foo ^= static_cast<bool>(p);<br>
<br>
in order to leave the interpretation of the expression unchanged.<br>
<br>
At a later date, more heuristics can be added so that the expression<br>
is something more idiomatic than static_cast, but that isn't an issue<br>
of correctness, it's an issue of style and hence subject to all kinds<br>
of varying opinions.<br>
<br>
At this point I am only concerned about correctness.<br></blockquote><div><br>Sure, but your transformation is stylistic in nature - so it's not just important to preserve semantics, but also to improve readability. Perhaps it's worth not offering a fixit hint in these cases, until good quality ones can be provided? I'm not sure.<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5">--<br>
"The Direct3D Graphics Pipeline" free book <<a href="http://tinyurl.com/d3d-pipeline" target="_blank">http://tinyurl.com/d3d-pipeline</a>><br>
The Computer Graphics Museum <<a href="http://ComputerGraphicsMuseum.org" target="_blank">http://ComputerGraphicsMuseum.org</a>><br>
The Terminals Wiki <<a href="http://terminals.classiccmp.org" target="_blank">http://terminals.classiccmp.org</a>><br>
Legalize Adulthood! (my blog) <<a href="http://LegalizeAdulthood.wordpress.com" target="_blank">http://LegalizeAdulthood.wordpress.com</a>><br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
</div></div></blockquote></div><br></div></div>