# Code Review: Ruby custom actions, various bug fixes

**URL:** <https://www.ruby-forum.com/t/code-review-ruby-custom-actions-various-bug-fixes/136664>\
**Category:** IronRuby\
**Created:** [May 5, 2008, 10:11am UTC](https://www.ruby-forum.com/t/code-review-ruby-custom-actions-various-bug-fixes/136664 "2008-05-05T10:11:47Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![grujildo](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/g/94ad74/32.png) [@grujildo](https://www.ruby-forum.com/u/grujildo)\
**Post date:** [May 5, 2008, 10:11am UTC](https://www.ruby-forum.com/t/code-review-ruby-custom-actions-various-bug-fixes/136664/1 "2008-05-05T10:11:47Z")

</div>

tfpt review /shelveset:Super17;REDMOND\tomat

DLR changes:

- 

```
     Removes InvokeMethodAttributes.

```

- 

```
     Makes ActionExpression factory public - it's needed by a 

```

custom Ruby action (ProtocolConverterAction).

Ruby changes:

- 

```
     Adds InvokeSuperMemberAction custom action

```

- 

```
     Refactors handling of member invocation in RubyBinder.

```

- 

```
     Improves implementation of super call. This is how 'super' 

```

seems to work:

1. 

```
   Let s be the inner-most scope in the lexical scope hierarchy 

```

of the ‘super’ call site that is either

a. a method scope, or

b. a block definition scope of a block whose caller is a method  
created by ‘define\_method’

1. 

```
  If s is a method definition scope (1a) then this method's frame 

```

f is available: either it is

a. the frame that invokes the ‘super’ call, or

b. captured by a closure of the block that contains the ‘super’  
call

1. 

```
  If s is a block (1b) then its caller's frame f is available.

         Use the arguments, self object and method name of the 

```

frame f for the ‘super’ invocation.

Implementation: the method defined by define\_method passes its frame  
into the block it calls (in BlockParam object). Super call site  
traverses lexical hierarchy and finds scope s and frame f. All  
information necessary to perform the super call are on f. For super to  
work within eval some additional work needs to be done beyond this  
shelveset - arguments needs to be distinguishable from other local  
variables at run-time. Super in eval will then do dynamic local variable  
lookup on the argument names that it finds in the frame f. This change  
doesn’t implement any optimizations though they are possible in some  
cases.

- 

```
     Adds ProtocolConverterAction custom action.

```

o It’s an abstract class for specific protocol conversions (to\_proc,  
to\_s, …)

o This shelveset adds only ConvertToProcAction \<:  
ProtocolConverterAction so far, we can replace Protocols by these  
actions in future.

o The protocol used for conversions is:

- 

```
    if obj.respond_to? :to_xxx

```

result = obj.to\_xxx  
if result.subclass\_of Xxx then return result  
end  
errror

o The action implements an optimization that avoids 2 dynamic  
dispatches in case respond\_to? is not overridden:

- 

```
    check class version of object obj in rule test

```

- 

```
    obj.class overrides respond_to? => no optimization (emit 2 

```

dynamic dispatches to respond\_to? and to\_xxx)

- 

```
    otherwise: lookup to_xxx, not found => emit error, found => 

```

emit static call to to\_xxx

- 

```
    check result of to_xxx

```

- 

```
     Implements to_proc conversion for & operator using 

```

ConvertToProcAction (fixes bug #19714: & operator should call to\_proc)

- 

```
     Adds #method-definition closure local variable of type 

```

RubyMethodInfo:

o Each method definition stores itself into this variable, which is  
used inside the method definition. This effectively enables any  
library/ops code to access the current method definition.

o RubyMethodInfo now holds on the method’s declaring local scope.

o Adds DefinitionName to RubyMethodInfo - this is the name of the  
method definition. The method itself could be used under different names  
(if aliased). Some language operations care only about definition name  
(like super call).

- 

```
     Adds UndefineMethod attribute - it is used on library classes 

```

to undefined methods defined in base classes.

- 

```
     Fixes GetInstanceMethods to skip undefined method.

```

- 

```
     Implements Kernel#block_given?, Module#module_function 

```

methods. Fixes Kernel#private/public/protected methods.

- 

```
     Replaces VariableExpression and ParameterExpression by 

```

Expression where possible.

- 

```
     Simplifies code emitted per method definition.

```

- 

```
     Fixes bug #19907: class_eval not singleton.

```

- 

```
     Fixes bug #19672: Logical operator in method arguments won't 

```

parse.

- 

```
     Fixes bug #19803: rescue next syntax bug.

```

- 

```
     Fixes bug #19892: Proc cannot be used as Boolean.

```

- 

```
     Fixes bug #19706: alias method resolution bug.

```

- 

```
     Adds more unit tests and refactors unit test driver a little 

```

bit.

Tomas

---

<div class="post-metadata">

**Author:** ![grujildo](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/g/94ad74/32.png) [@grujildo](https://www.ruby-forum.com/u/grujildo)\
**Post date:** [May 5, 2008, 4:51pm UTC](https://www.ruby-forum.com/t/code-review-ruby-custom-actions-various-bug-fixes/136664/2 "2008-05-05T16:51:51Z")

</div>

DLR changes are great!

From: Tomas M.  
Sent: Monday, May 05, 2008 12:54 AM  
To: IronRuby External Code R.; Rowan Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Code Review: Ruby custom actions, various bug fixes

tfpt review /shelveset:Super17;REDMOND\tomat

DLR changes:

- 

```
     Removes InvokeMethodAttributes.

```

- 

```
     Makes ActionExpression factory public - it's needed by a 

```

custom Ruby action (ProtocolConverterAction).

Ruby changes:

- 

```
     Adds InvokeSuperMemberAction custom action

```

- 

```
     Refactors handling of member invocation in RubyBinder.

```

- 

```
     Improves implementation of super call. This is how 'super' 

```

seems to work:

1. 

```
   Let s be the inner-most scope in the lexical scope hierarchy 

```

of the ‘super’ call site that is either

a. a method scope, or

b. a block definition scope of a block whose caller is a method  
created by ‘define\_method’

1. 

```
  If s is a method definition scope (1a) then this method's frame 

```

f is available: either it is

a. the frame that invokes the ‘super’ call, or

b. captured by a closure of the block that contains the ‘super’  
call

1. 

```
  If s is a block (1b) then its caller's frame f is available.

         Use the arguments, self object and method name of the 

```

frame f for the ‘super’ invocation.

Implementation: the method defined by define\_method passes its frame  
into the block it calls (in BlockParam object). Super call site  
traverses lexical hierarchy and finds scope s and frame f. All  
information necessary to perform the super call are on f. For super to  
work within eval some additional work needs to be done beyond this  
shelveset - arguments needs to be distinguishable from other local  
variables at run-time. Super in eval will then do dynamic local variable  
lookup on the argument names that it finds in the frame f. This change  
doesn’t implement any optimizations though they are possible in some  
cases.

- 

```
     Adds ProtocolConverterAction custom action.

```

o It’s an abstract class for specific protocol conversions (to\_proc,  
to\_s, …)

o This shelveset adds only ConvertToProcAction \<:  
ProtocolConverterAction so far, we can replace Protocols by these  
actions in future.

o The protocol used for conversions is:

- 

```
    if obj.respond_to? :to_xxx

```

result = obj.to\_xxx  
if result.subclass\_of Xxx then return result  
end  
errror

o The action implements an optimization that avoids 2 dynamic  
dispatches in case respond\_to? is not overridden:

- 

```
    check class version of object obj in rule test

```

- 

```
    obj.class overrides respond_to? => no optimization (emit 2 

```

dynamic dispatches to respond\_to? and to\_xxx)

- 

```
    otherwise: lookup to_xxx, not found => emit error, found => 

```

emit static call to to\_xxx

- 

```
    check result of to_xxx

```

- 

```
     Implements to_proc conversion for & operator using 

```

ConvertToProcAction (fixes bug #19714: & operator should call to\_proc)

- 

```
     Adds #method-definition closure local variable of type 

```

RubyMethodInfo:

o Each method definition stores itself into this variable, which is  
used inside the method definition. This effectively enables any  
library/ops code to access the current method definition.

o RubyMethodInfo now holds on the method’s declaring local scope.

o Adds DefinitionName to RubyMethodInfo - this is the name of the  
method definition. The method itself could be used under different names  
(if aliased). Some language operations care only about definition name  
(like super call).

- 

```
     Adds UndefineMethod attribute - it is used on library classes 

```

to undefined methods defined in base classes.

- 

```
     Fixes GetInstanceMethods to skip undefined method.

```

- 

```
     Implements Kernel#block_given?, Module#module_function 

```

methods. Fixes Kernel#private/public/protected methods.

- 

```
     Replaces VariableExpression and ParameterExpression by 

```

Expression where possible.

- 

```
     Simplifies code emitted per method definition.

```

- 

```
     Fixes bug #19907: class_eval not singleton.

```

- 

```
     Fixes bug #19672: Logical operator in method arguments won't 

```

parse.

- 

```
     Fixes bug #19803: rescue next syntax bug.

```

- 

```
     Fixes bug #19892: Proc cannot be used as Boolean.

```

- 

```
     Fixes bug #19706: alias method resolution bug.

```

- 

```
     Adds more unit tests and refactors unit test driver a little 

```

bit.

Tomas

---

<div class="post-metadata">

**Author:** ![grujildo](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/g/94ad74/32.png) [@grujildo](https://www.ruby-forum.com/u/grujildo)\
**Post date:** [May 5, 2008, 6:48pm UTC](https://www.ruby-forum.com/t/code-review-ruby-custom-actions-various-bug-fixes/136664/3 "2008-05-05T18:48:09Z")

</div>

Ruby changes look good.

I’ve got one comment based upon where the DLR is going with actions -  
not that you need to change anything now, just that it might make your  
life easier in the future if you find yourself adding more actions.  
That is for some actions (e.g. ProtocolConversionAction) you have the  
rule production logic in the action it’s self. For others (e.g. Super)  
that logic lives in the RubyBinder. Keeping all of it in the action’s  
might make it easier when the DLR switches to having the action’s (then  
called SiteBinders or whatever the final name is) produce the rule. Of  
course you could always choose to forward them onto the RubyBinder  
yourself.

From: Tomas M.  
Sent: Monday, May 05, 2008 12:54 AM  
To: IronRuby External Code R.; Rowan Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Code Review: Ruby custom actions, various bug fixes

tfpt review /shelveset:Super17;REDMOND\tomat

DLR changes:

- 

```
     Removes InvokeMethodAttributes.

```

- 

```
     Makes ActionExpression factory public - it's needed by a 

```

custom Ruby action (ProtocolConverterAction).

Ruby changes:

- 

```
     Adds InvokeSuperMemberAction custom action

```

- 

```
     Refactors handling of member invocation in RubyBinder.

```

- 

```
     Improves implementation of super call. This is how 'super' 

```

seems to work:

1. 

```
   Let s be the inner-most scope in the lexical scope hierarchy 

```

of the ‘super’ call site that is either

a. a method scope, or

b. a block definition scope of a block whose caller is a method  
created by ‘define\_method’

1. 

```
  If s is a method definition scope (1a) then this method's frame 

```

f is available: either it is

a. the frame that invokes the ‘super’ call, or

b. captured by a closure of the block that contains the ‘super’  
call

1. 

```
  If s is a block (1b) then its caller's frame f is available.

         Use the arguments, self object and method name of the 

```

frame f for the ‘super’ invocation.

Implementation: the method defined by define\_method passes its frame  
into the block it calls (in BlockParam object). Super call site  
traverses lexical hierarchy and finds scope s and frame f. All  
information necessary to perform the super call are on f. For super to  
work within eval some additional work needs to be done beyond this  
shelveset - arguments needs to be distinguishable from other local  
variables at run-time. Super in eval will then do dynamic local variable  
lookup on the argument names that it finds in the frame f. This change  
doesn’t implement any optimizations though they are possible in some  
cases.

- 

```
     Adds ProtocolConverterAction custom action.

```

o It’s an abstract class for specific protocol conversions (to\_proc,  
to\_s, …)

o This shelveset adds only ConvertToProcAction \<:  
ProtocolConverterAction so far, we can replace Protocols by these  
actions in future.

o The protocol used for conversions is:

- 

```
    if obj.respond_to? :to_xxx

```

result = obj.to\_xxx  
if result.subclass\_of Xxx then return result  
end  
errror

o The action implements an optimization that avoids 2 dynamic  
dispatches in case respond\_to? is not overridden:

- 

```
    check class version of object obj in rule test

```

- 

```
    obj.class overrides respond_to? => no optimization (emit 2 

```

dynamic dispatches to respond\_to? and to\_xxx)

- 

```
    otherwise: lookup to_xxx, not found => emit error, found => 

```

emit static call to to\_xxx

- 

```
    check result of to_xxx

```

- 

```
     Implements to_proc conversion for & operator using 

```

ConvertToProcAction (fixes bug #19714: & operator should call to\_proc)

- 

```
     Adds #method-definition closure local variable of type 

```

RubyMethodInfo:

o Each method definition stores itself into this variable, which is  
used inside the method definition. This effectively enables any  
library/ops code to access the current method definition.

o RubyMethodInfo now holds on the method’s declaring local scope.

o Adds DefinitionName to RubyMethodInfo - this is the name of the  
method definition. The method itself could be used under different names  
(if aliased). Some language operations care only about definition name  
(like super call).

- 

```
     Adds UndefineMethod attribute - it is used on library classes 

```

to undefined methods defined in base classes.

- 

```
     Fixes GetInstanceMethods to skip undefined method.

```

- 

```
     Implements Kernel#block_given?, Module#module_function 

```

methods. Fixes Kernel#private/public/protected methods.

- 

```
     Replaces VariableExpression and ParameterExpression by 

```

Expression where possible.

- 

```
     Simplifies code emitted per method definition.

```

- 

```
     Fixes bug #19907: class_eval not singleton.

```

- 

```
     Fixes bug #19672: Logical operator in method arguments won't 

```

parse.

- 

```
     Fixes bug #19803: rescue next syntax bug.

```

- 

```
     Fixes bug #19892: Proc cannot be used as Boolean.

```

- 

```
     Fixes bug #19706: alias method resolution bug.

```

- 

```
     Adds more unit tests and refactors unit test driver a little 

```

bit.

Tomas
