[erlang-patches] [erlang-bugs] Use a set to store ref registers in beam_receive

[Björn Gustavsson]

Looks good! Good work on the test case.

We will test for a few days in our daily builds
before merging it to maint.


On Thu, Apr 11, 2013 at 8:05 PM, Anthony Ramine <n.oxyde@REDACTED> wrote:

> I added a test and clarified the code following your suggestions.
>
> I noticed that RefReg wasn't a descriptive enough name but I am ashamed to
> admit I optimized for diff shortness, sorry.
>
> Regards,
>
> --
> Anthony Ramine
>
> Le 11 avr. 2013 à 16:01, Björn Gustavsson a écrit :
>
> > On Wed, Apr 10, 2013 at 12:22 PM, Anthony Ramine <n.oxyde@REDACTED>
> wrote:
> > Hello Björn,
> >
> > I'm not sure I understand what you mean, should I add my example
> function into receive_SUITE.erl as is to test that? I'm not sure that would
> demonstrate anything in the long term as my other cooking patch
> move-let-into-seq makes the culprit code where a reference is live in two Y
> registers disappear.
> >
> >
> > OK. I realize that it is hard or impossible to
> > construct a test case that would test something
> > that is not already tested by existing test cases.
> >
> > Your patch correctly fixes the bug, but I
> > have some comments and suggestions
> > for further simplification:
> >
> > The name of the RefReg variable is now
> > misleading, since it contains a register set.
> > (Suggested new name: RefRegSet, RefRegs,
> > or RefSet.)
> >
> > The comment for opt_ref_used/4 needs to
> > be updated.
> >
> > In opt_recv/5, my original code looked like:
> >
> >     case regs_to_list(R) of
> >       [{y,_}=RefReg] -> ...
> >
> > The matching of {y,_} is just a cheap
> > assertion (only added because it was
> > almost free).
> >
> > Since your new code sends the register
> > set to the opt_ref_used/4, there is no
> > longer any need to convert the register
> > set to a list. Thus we can write:
> >
> >   case regs_empty(R) of
> >      false -> ...
> >
> > and remove the regs_to_list/1 function.
> >
> > Finally, for clarity I would add parenthesis
> > in is_ref_msg_comparison/3:
> >
> > is_ref_msg_comparison([R1,R2], RefReg, Regs) ->
> >     (regs_is_member(R2, RefReg) andalso regs_is_member(R1, Regs)) orelse
> >     (regs_is_member(R1, RefReg) andalso regs_is_member(R2, Regs)).
> >
> >
> > --
> > Björn Gustavsson, Erlang/OTP, Ericsson AB
>
>


-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20130412/388dd8af/attachment.htm>