tfpt review "/shelveset:MoreSites4;REDMOND\tomat" DLR change: Inherits SiteLocalStorage<T> from SiteLocalStorage class and enables any subclass of SiteLocalStorage to be created as a storage for data in a rule. Ruby: Implements multiple SiteLocalStorage subclasses for the most frequent site storage types: CallSiteStorage, BinaryOpStorage, UnaryOpStorage, ConversionStorage. Implements ConvertToHashAction, fixes implementation of ConvertToSAction. Conversion actions now use RubyContext not RubyScope. This simplifies their calls from libraries yet causes a minor incompatibility with MRI: if a parameter-less method that works with a local scope, such as "private", is aliased and given the name of a conversion method ("to_s", e.g.) and a conversion of an object with such aliased method is attempted, the scope-manipulating method fails to update the scope properly. This is not a real world scenario so it should be fine to ignore it. The fix would require to define 2 instances of each protocol conversion action - one HasScope flag and the other without it. Removes more static sites. Tomas -------------- next part -------------- A non-text attachment was scrubbed... Name: MoreSites4.diff Type: application/octet-stream Size: 748315 bytes Desc: MoreSites4.diff URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20081209/0c39751f/attachment-0001.obj>
DLR looks good -----Original Message----- From: Tomas Matousek Sent: Tuesday, December 09, 2008 6:16 PM To: IronRuby External Code Reviewers; DLR Code Reviews Cc: ironruby-core at rubyforge.org Subject: Code Review: MoreSites4 tfpt review "/shelveset:MoreSites4;REDMOND\tomat" DLR change: Inherits SiteLocalStorage<T> from SiteLocalStorage class and enables any subclass of SiteLocalStorage to be created as a storage for data in a rule. Ruby: Implements multiple SiteLocalStorage subclasses for the most frequent site storage types: CallSiteStorage, BinaryOpStorage, UnaryOpStorage, ConversionStorage. Implements ConvertToHashAction, fixes implementation of ConvertToSAction. Conversion actions now use RubyContext not RubyScope. This simplifies their calls from libraries yet causes a minor incompatibility with MRI: if a parameter-less method that works with a local scope, such as "private", is aliased and given the name of a conversion method ("to_s", e.g.) and a conversion of an object with such aliased method is attempted, the scope-manipulating method fails to update the scope properly. This is not a real world scenario so it should be fine to ignore it. The fix would require to define 2 instances of each protocol conversion action - one HasScope flag and the other without it. Removes more static sites. Tomas
Ruby changes look good. -----Original Message----- From: Tomas Matousek Sent: Tuesday, December 09, 2008 6:16 PM To: IronRuby External Code Reviewers; DLR Code Reviews Cc: ironruby-core at rubyforge.org Subject: Code Review: MoreSites4 tfpt review "/shelveset:MoreSites4;REDMOND\tomat" DLR change: Inherits SiteLocalStorage<T> from SiteLocalStorage class and enables any subclass of SiteLocalStorage to be created as a storage for data in a rule. Ruby: Implements multiple SiteLocalStorage subclasses for the most frequent site storage types: CallSiteStorage, BinaryOpStorage, UnaryOpStorage, ConversionStorage. Implements ConvertToHashAction, fixes implementation of ConvertToSAction. Conversion actions now use RubyContext not RubyScope. This simplifies their calls from libraries yet causes a minor incompatibility with MRI: if a parameter-less method that works with a local scope, such as "private", is aliased and given the name of a conversion method ("to_s", e.g.) and a conversion of an object with such aliased method is attempted, the scope-manipulating method fails to update the scope properly. This is not a real world scenario so it should be fine to ignore it. The fix would require to define 2 instances of each protocol conversion action - one HasScope flag and the other without it. Removes more static sites. Tomas