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

[Björn Gustavsson]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20130327/bc5d6bf2/attachment.htm>