tfpt review "/shelveset:RuntimeBoundRules;REDMOND\tomat" Comment : Implements atomizer for Ruby call site binders: A new class RubyMetaBinderFactory creates sites and caches them in dictionaries. Sites are either bound to a runtime (RubyContext) or not. Bound sites don''t emit context check, unbound need to. Bound sites don''t take RubyContext as the first parameter any more. They can still take RubyScope if needed (such sites have HasScope flag set). Replaces SiteLocalStorage with RubyCallSiteStorage that holds on the context so that it is not necessary to pass RubyContext to the sites stored in the storage. Updates all sites in libraries accordingly. Adds -ER command line option that enables tracing of meta-objects (rules) created by Ruby. Refactors MetaObjectBuilder to use type restrictions rather than expression restrictions. Removes RubyClassAttribute.MixinInterfaces - it didn''t work correctly and was almost unused. Also improves handling of BOM in 1.8 mode - we previously did exactly what Ruby does, i.e. report a syntax error from which it is not obvious what''s wrong. If -KU is not specified and UTF8 BOM is found at the beginning of the source code, the tokenizer reports a level 2 warning, skips the BOM as if it was whitespace and continues reading the file. Tomas -------------- next part -------------- A non-text attachment was scrubbed... Name: RuntimeBoundRules.diff Type: application/octet-stream Size: 797016 bytes Desc: RuntimeBoundRules.diff URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20090325/f88267b4/attachment-0001.obj>
Looks good! I like the use of Key.Create to allow construction of a templated class without providing the template types :). But why is Key<,> a reference type instead of a value type? And I think the name "Pair" would be better than the name "Key"... "BOM can be used as an identifier in 1.8 -KU mode" -- good grief! -----Original Message----- From: Tomas Matousek Sent: Wednesday, March 25, 2009 8:50 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: Code Review: RuntimeBoundRules tfpt review "/shelveset:RuntimeBoundRules;REDMOND\tomat" Comment : Implements atomizer for Ruby call site binders: A new class RubyMetaBinderFactory creates sites and caches them in dictionaries. Sites are either bound to a runtime (RubyContext) or not. Bound sites don''t emit context check, unbound need to. Bound sites don''t take RubyContext as the first parameter any more. They can still take RubyScope if needed (such sites have HasScope flag set). Replaces SiteLocalStorage with RubyCallSiteStorage that holds on the context so that it is not necessary to pass RubyContext to the sites stored in the storage. Updates all sites in libraries accordingly. Adds -ER command line option that enables tracing of meta-objects (rules) created by Ruby. Refactors MetaObjectBuilder to use type restrictions rather than expression restrictions. Removes RubyClassAttribute.MixinInterfaces - it didn''t work correctly and was almost unused. Also improves handling of BOM in 1.8 mode - we previously did exactly what Ruby does, i.e. report a syntax error from which it is not obvious what''s wrong. If -KU is not specified and UTF8 BOM is found at the beginning of the source code, the tokenizer reports a level 2 warning, skips the BOM as if it was whitespace and continues reading the file. Tomas
As Dino pointed out if Key was a struct Dictionary<Key, X> would be jitted for each instantiation of Key. I didn''t named it "Pair", since we might need triples, quadruples, etc. as well (i.e. Key<,,>, Key<,,,>, etc.) We might use Tuple in future - if it had Equals and GetHashCode implemented. Tomas -----Original Message----- From: Curt Hagenlocher Sent: Thursday, March 26, 2009 9:08 AM To: Tomas Matousek; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Code Review: RuntimeBoundRules Looks good! I like the use of Key.Create to allow construction of a templated class without providing the template types :). But why is Key<,> a reference type instead of a value type? And I think the name "Pair" would be better than the name "Key"... "BOM can be used as an identifier in 1.8 -KU mode" -- good grief! -----Original Message----- From: Tomas Matousek Sent: Wednesday, March 25, 2009 8:50 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: Code Review: RuntimeBoundRules tfpt review "/shelveset:RuntimeBoundRules;REDMOND\tomat" Comment : Implements atomizer for Ruby call site binders: A new class RubyMetaBinderFactory creates sites and caches them in dictionaries. Sites are either bound to a runtime (RubyContext) or not. Bound sites don''t emit context check, unbound need to. Bound sites don''t take RubyContext as the first parameter any more. They can still take RubyScope if needed (such sites have HasScope flag set). Replaces SiteLocalStorage with RubyCallSiteStorage that holds on the context so that it is not necessary to pass RubyContext to the sites stored in the storage. Updates all sites in libraries accordingly. Adds -ER command line option that enables tracing of meta-objects (rules) created by Ruby. Refactors MetaObjectBuilder to use type restrictions rather than expression restrictions. Removes RubyClassAttribute.MixinInterfaces - it didn''t work correctly and was almost unused. Also improves handling of BOM in 1.8 mode - we previously did exactly what Ruby does, i.e. report a syntax error from which it is not obvious what''s wrong. If -KU is not specified and UTF8 BOM is found at the beginning of the source code, the tokenizer reports a level 2 warning, skips the BOM as if it was whitespace and continues reading the file. Tomas