<div dir="ltr">Thanks for the patch!<div><br></div><div style>I have reviewed your patch and have a few comments:</div><div style><br></div><div style>The following line:</div><div style><br></div><div style>  Vs = [V||{_,#c_var{name=V}}<-Vars0]</div>
<div style><br></div><div style>should be written with spaces around || and <-:</div><div style><br></div><div style><div>  Vs = [V || {_,#c_var{name=V}} <- Vars0]</div><div><br></div><div style>to match the style in the rest of the compiler.</div>
<div style><br></div><div style><br></div><div style>The following is a matter of taste, but I would</div><div style>prefer to avoid giving mul_pairs() an extra argument</div><div style>just so that it can build it into the return tuple</div>
<div style>(and otherwise unused).</div><div style><br></div><div style>Thus, I would prefer bc_elem_size() to end like this:</div><div style><br></div><div style>  {E,Pre,St} = bc_mul_pairs(F, #c_literal{val=Bits}, [], St0),</div>
<div style>  {E,Pre,Vs,St}.</div><div style><br></div><div style>to avoid making bc_mul_pairs() harder to read.</div><div style><br></div><div style><br></div><div style>There are no test cases. Could you please add</div>
<div style>a test case (making sure that both of the new throw(impossible)</div><div style>in bc_verify_non_filtering/1 are covered)?</div><div style><br></div><div style>bc_bincomp_SUITE.erl should be a good place.</div>
<div style>(Remember, new test cases should not have any ?line</div><div style>macros addes.)</div><div style><br></div><div style>/Bjorn</div><div style><br></div></div><div style><br></div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Wed, Mar 27, 2013 at 10:09 AM, Anthony Ramine <span dir="ltr"><<a href="mailto:n.oxyde@gmail.com" target="_blank">n.oxyde@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<br>
<br>
I amended my second commit and to make +clint0 run the Core Erlang linting pass just before any optimisation pass.<br>
<br>
Please refetch.<br>
<br>
Regards,<br>
<br>
--<br>
Anthony Ramine<br>
<br>
Le 25 mars 2013 à 10:08, Fredrik a écrit :<br>
<div class="im"><br>
> On 03/24/2013 06:50 PM, Anthony Ramine wrote:<br>
</div>>> git fetch <a href="https://github.com/nox/otp.git" target="_blank">https://github.com/nox/otp.git</a> fix-bc-optim<br>
<div class="HOEnZb"><div class="h5">> Fetched and currently in the 'pu' branch.<br>
> Thanks,<br>
><br>
> --<br>
><br>
> BR Fredrik Gustafsson<br>
> Erlang OTP Team<br>
><br>
<br>
_______________________________________________<br>
erlang-patches mailing list<br>
<a href="mailto:erlang-patches@erlang.org">erlang-patches@erlang.org</a><br>
<a href="http://erlang.org/mailman/listinfo/erlang-patches" target="_blank">http://erlang.org/mailman/listinfo/erlang-patches</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>Björn Gustavsson, Erlang/OTP, Ericsson AB
</div>