<div dir="ltr">I think this is a matter of opinion and the code is fine as is personally.  If you prefer the implicit cast, feel free to leave it.  This pattern is already pervasive all over LLVM and clang, so you can treat it as a nit (fix it if you agree, leave it if you prefer).</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018 at 9:57 AM Stella Stamenova via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">stella.stamenova marked 8 inline comments as done.<br>
stella.stamenova added a comment.<br>
<br>
I made the requested changes and I'll update the revision once all the tests pass.<br>
<br>
<br>
<br>
================<br>
Comment at: source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp:92<br>
+    Unwind *unwinder = GetUnwinder();<br>
+    if (unwinder)<br>
+      reg_ctx_sp = unwinder->CreateRegisterContextForFrame(frame);<br>
----------------<br>
tatyana-krasnukha wrote:<br>
> aleksandr.urakov wrote:<br>
> > tatyana-krasnukha wrote:<br>
> > > Here you check plain pointer, don't cast it to bool - compare with nullptr.<br>
> > What are the disadvantages of casting to bool in comparison with nullptr checking?<br>
> That's a matter of opinion, but most coding styles consider implicit casts as less readable. E.g. if you run clang-tidy with enabled readability checks, you get readability-implicit-bool-conversion warning on this code.<br>
I actually intentionally copied this line and several of the others verbatim from the other targets (so that they are the same) but I can fix both places.<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D49111" rel="noreferrer" target="_blank">https://reviews.llvm.org/D49111</a><br>
<br>
<br>
<br>
</blockquote></div>