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

[Anthony Ramine]

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