Inline. From: Dino Viehland Sent: Thursday, June 18, 2009 12:55 PM To: Tomas Matousek; IronRuby External Code Reviewers; 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 Matousek Sent: Thursday, June 18, 2009 11:33 AM To: IronRuby External Code Reviewers; Rowan Code Reviewers Cc: [email protected] 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
_______________________________________________ Ironruby-core mailing list [email protected] http://rubyforge.org/mailman/listinfo/ironruby-core
