Inline.
From: Dino Viehland
Sent: Thursday, June 18, 2009 12:55 PM
To: Tomas M.; IronRuby External Code R.; Rowan Code
Reviewers
Cc: [email protected]
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: [email protected]
Subject: Code Review: AdaptiveRules4
tfpt review “/shelveset:AdaptiveRules4;REDMOND\tomat”
DLR: Fixes bugs and implements new features in interpreter.
encountered.
Ruby:
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.
Tomas