# Code Review: AdaptiveRules4

**URL:** <https://www.ruby-forum.com/t/code-review-adaptiverules4/170551>\
**Category:** IronRuby\
**Created:** [June 18, 2009, 8:49pm UTC](https://www.ruby-forum.com/t/code-review-adaptiverules4/170551 "2009-06-18T20:49:28Z")\
**Posts on this page:** 2\
**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:** [June 18, 2009, 8:49pm UTC](https://www.ruby-forum.com/t/code-review-adaptiverules4/170551/1 "2009-06-18T20:49:28Z")

</div>

tfpt review “/shelveset:AdaptiveRules4;REDMOND\tomat”

DLR: Fixes bugs and implements new features in interpreter.

- 

```
     Static field assignment.

```

- 

```
     LessThan

```

- 

```
     ConvertUnary - should re-box numeric values.

```

- 

```
     Force compilation whenever ref/out parameters are 

```

encountered.

- 

```
     Invocation expression with delegate target.

```

- 

```
     Unbox - no-op.

```

- 

```
     TypeEqual and TypeIs for sealed types.

```

Ruby:

- 

```
     Implements adaptively compiled rules:

```

- 

```
     RubyMetaBinder (subclass of all Ruby binders) implements 

```

BindDelegate so that:

o It creates AST for the rule calling Bind and wraps the result into a  
lambda exactly like call site binder does.

o If this lambda is interpretable (doesn’t contain loops or other  
constructs that force compilation) it creates an InterpretedDispatcher  
that hooks the lambda’s “Compiled” event so that it gets called back as  
soon as compiled delegate is available. Until that happens it dispatches  
rule invocations to the interpreter.

o If the lambda is forced-compiled it uses the resulting delegate  
right away.

o The dispatcher is a generic class generated for Func and Action  
delegates with 0…15 generic parameters.

- 

```
     Also Adds specs for "include?" used on ClrNames.

```

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:** [June 19, 2009, 3:47pm UTC](https://www.ruby-forum.com/t/code-review-adaptiverules4/170551/2 "2009-06-19T15:47:15Z")

</div>

Inline.

From: Dino Viehland  
Sent: Thursday, June 18, 2009 12:55 PM  
To: Tomas M.; IronRuby External Code R.; Rowan Code  
Reviewers  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Code Review: AdaptiveRules4

On the rethrow in NewInstruction you should do  
ExceptionHelpers.UpdateForRethrow (otherwise the stack trace for the  
inner exception will be lost).

OK.

I would suggest that we should generate the numeric conversions into  
specialized subclasses so we don’t need to do the switch at runtime.

I think switch is not as expensive here given that we are  
boxing/unboxing already. The number of classes would be huge (there are  
11 x 4 x 2 = 88 cases). So I would prefer to keep the switch.

On CompileSetItemArray is it the case that this is really not  
implemented? Can you actually write something in the TODO: and remove  
the code which presumably doesn’t work?

Yes it is not implemented. I’ll do that.

Can you throw NotImplemented in CompileConvertToType instead of just  
returning when we don’t support the conversion?

TODO here doesn’t mean that the conversion is not supported. For example  
conversions to a super-class or implemented interfaces are no-op. A  
conversion to a non-implemented interface or an unrelated class, etc.  
should fail. So this needs to be fixed, but I can’t just throw an  
exception since in most cases it is ok. Will add better comment.

What was wrong w/ StaticFieldAccessInstruction? Why do we want to have  
to push and pop an extra value when accessing a static field?

I though static field access is not so frequent operation to be  
optimized, so we can save an instruction class. I can roll it back if  
you think we should optimize that case.

Otherwise the outer layer changes look good.

From: Tomas M.  
Sent: Thursday, June 18, 2009 11:33 AM  
To: IronRuby External Code R.; Rowan Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Code Review: AdaptiveRules4

tfpt review “/shelveset:AdaptiveRules4;REDMOND\tomat”

DLR: Fixes bugs and implements new features in interpreter.

- 

```
     Static field assignment.

```

- 

```
     LessThan

```

- 

```
     ConvertUnary - should re-box numeric values.

```

- 

```
     Force compilation whenever ref/out parameters are 

```

encountered.

- 

```
     Invocation expression with delegate target.

```

- 

```
     Unbox - no-op.

```

- 

```
     TypeEqual and TypeIs for sealed types.

```

Ruby:

- 

```
     Implements adaptively compiled rules:

```

- 

```
     RubyMetaBinder (subclass of all Ruby binders) implements 

```

BindDelegate so that:

o It creates AST for the rule calling Bind and wraps the result into a  
lambda exactly like call site binder does.

o If this lambda is interpretable (doesn’t contain loops or other  
constructs that force compilation) it creates an InterpretedDispatcher  
that hooks the lambda’s “Compiled” event so that it gets called back as  
soon as compiled delegate is available. Until that happens it dispatches  
rule invocations to the interpreter.

o If the lambda is forced-compiled it uses the resulting delegate  
right away.

o The dispatcher is a generic class generated for Func and Action  
delegates with 0…15 generic parameters.

- 

```
     Also Adds specs for "include?" used on ClrNames.

```

Tomas
