[erlang-patches] Fix a bug in some binary comprehensions

Anthony Ramine n.oxyde@REDACTED
Thu Mar 28 11:36:22 CET 2013


Hello Björn,

I amended my patch with the modifications you suggested; please refetch.

Regards,

-- 
Anthony Ramine

Le 27 mars 2013 à 15:43, Björn Gustavsson a écrit :

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




More information about the erlang-patches mailing list