<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 29, 2013 at 2:09 PM, Stephen Kelly <span dir="ltr"><<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.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"><div class=""><div class="h5">Nicolas Desprès wrote:<br>
<br>
> On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly<br>
> <<a href="mailto:steveire@gmail.com">steveire@gmail.com</a>> wrote:<br>
><br>
>> Nicolas Desprès wrote:<br>
>> > It was fastest because it was not doing the right thing. I tried to<br>
>> > patch it properly and the benchmark are the same whether we use the<br>
>> > default comparison functor or mine.<br>
>> ><br>
>> > So I think you can merge it like that. I have pushed a new version<br>
>> without<br>
>> > the comment.<br>
>> ><br>
>><br>
>> I still haven't tried it, but there are still style issues.<br>
><br>
><br>
>> * Don't put an else after a return<br>
>> * Wrap single line blocks in {}<br>
>><br>
><br>
> Fixed and force-pushed. Sorry for the inconvenience. I am not used to this<br>
> style yet.<br>
<br>
</div></div>Your Compare::operator() contains this:<br>
<br>
 if (j == s2.rend())<br>
   {<br>
   return false;<br>
   }<br>
 return false;<br>
<br>
Any reason not to simplify that?<br></blockquote><div style>Because I used to have a comment in the else branch of the if, that I removed because part of it was wrong <a href="https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023">https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023</a> (the part about the optimization). Then I kept the code without thinking more about it.</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>
Also, I don't see why the custom comparison functor is needed at all. I<br>
removed it and sped up the test significantly. Can you explain?<br></blockquote><div><br></div><div style>I had some failing tests because the previous algorithm was not a plain strcpy().</div><div style><br></div><div style>

I have restored the comment. I prefer to keep the too branch even if both return false to make it clear that the comment apply only in this case and that it is the only difference with a normal strcpy(3) algorithm.</div>
<div style>
</div></div><div class="gmail_extra"><br></div><div class="gmail_extra" style>Cheers,</div><div class="gmail_extra"><br></div>-- <br>Nicolas Desprès<br>
</div></div>