<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=us-ascii">
<meta name=Generator content="Microsoft Word 12 (filtered medium)">
<style>
<!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.5pt;
font-family:Consolas;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:Consolas;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page Section1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.Section1
{page:Section1;}
/* List Definitions */
@list l0
{mso-list-id:13119758;
mso-list-type:hybrid;
mso-list-template-ids:-1757409170 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l0:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:.75in;
text-indent:-.25in;}
@list l0:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:1.25in;
text-indent:-.25in;}
@list l0:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
margin-left:1.75in;
text-indent:-9.0pt;}
@list l0:level4
{mso-level-tab-stop:2.25in;
mso-level-number-position:left;
margin-left:2.25in;
text-indent:-.25in;}
@list l0:level5
{mso-level-tab-stop:2.75in;
mso-level-number-position:left;
margin-left:2.75in;
text-indent:-.25in;}
@list l0:level6
{mso-level-tab-stop:3.25in;
mso-level-number-position:left;
margin-left:3.25in;
text-indent:-.25in;}
@list l0:level7
{mso-level-tab-stop:3.75in;
mso-level-number-position:left;
margin-left:3.75in;
text-indent:-.25in;}
@list l0:level8
{mso-level-tab-stop:4.25in;
mso-level-number-position:left;
margin-left:4.25in;
text-indent:-.25in;}
@list l0:level9
{mso-level-tab-stop:4.75in;
mso-level-number-position:left;
margin-left:4.75in;
text-indent:-.25in;}
@list l1
{mso-list-id:666830028;
mso-list-type:hybrid;
mso-list-template-ids:1943199658 -978678212 67698691 67698689 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l1:level1
{mso-level-start-at:2;
mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Calibri","sans-serif";
mso-fareast-font-family:Calibri;
mso-bidi-font-family:"Times New Roman";}
@list l1:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l1:level3
{mso-level-number-format:bullet;
mso-level-text:\F0B7;
mso-level-tab-stop:1.5in;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l1:level4
{mso-level-tab-stop:2.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level5
{mso-level-tab-stop:2.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level6
{mso-level-tab-stop:3.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level7
{mso-level-tab-stop:3.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level8
{mso-level-tab-stop:4.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level9
{mso-level-tab-stop:4.5in;
mso-level-number-position:left;
text-indent:-.25in;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
-->
</style>
<!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang=EN-US link=blue vlink=purple>
<div class=Section1>
<p class=MsoNormal><span style='color:#1F497D'>Ruby changes look good. <o:p></o:p></span></p>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<p class=MsoNormal><span style='color:#1F497D'>I’ve got one comment based upon
where the DLR is going with actions – not that you need to change anything now,
just that it might make your life easier in the future if you find yourself
adding more actions. That is for some actions (e.g. ProtocolConversionAction)
you have the rule production logic in the action it’s self. For others (e.g. Super)
that logic lives in the RubyBinder. Keeping all of it in the action’s might
make it easier when the DLR switches to having the action’s (then called
SiteBinders or whatever the final name is) produce the rule. Of course you
could always choose to forward them onto the RubyBinder yourself.<o:p></o:p></span></p>
<p class=MsoNormal><span style='color:#1F497D'><o:p> </o:p></span></p>
<div>
<div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'>
<p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span
style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> Tomas Matousek <br>
<b>Sent:</b> Monday, May 05, 2008 12:54 AM<br>
<b>To:</b> IronRuby External Code Reviewers; Rowan Code Reviewers<br>
<b>Cc:</b> ironruby-core@rubyforge.org<br>
<b>Subject:</b> Code Review: Ruby custom actions, various bug fixes<o:p></o:p></span></p>
</div>
</div>
<p class=MsoNormal><o:p> </o:p></p>
<p class=MsoPlainText>tfpt review /shelveset:Super17;REDMOND\tomat<o:p></o:p></p>
<p class=MsoPlainText><o:p> </o:p></p>
<p class=MsoNormal><b>DLR changes</b>: <o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Removes InvokeMethodAttributes.<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Makes ActionExpression factory public – it’s
needed by a custom Ruby action (ProtocolConverterAction).<o:p></o:p></p>
<p class=MsoNormal><o:p> </o:p></p>
<p class=MsoNormal><b>Ruby changes</b>:<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Adds InvokeSuperMemberAction custom action<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Refactors handling of member invocation in RubyBinder.<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Improves implementation of super call. This is how
‘super’ seems to work:<o:p></o:p></p>
<p class=MsoListParagraph><o:p> </o:p></p>
<p class=MsoListParagraph style='margin-left:.75in;text-indent:-.25in;
mso-list:l0 level1 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>1.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]> Let
<b>s</b> be the inner-most scope in the lexical scope hierarchy of the ‘super’
call site that is either <o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.25in;text-indent:-.25in;
mso-list:l0 level2 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>a.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]>a
method scope, or<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.25in;text-indent:-.25in;
mso-list:l0 level2 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>b.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]>a
block definition scope of a block whose caller is a method created by
‘define_method’<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:.75in;text-indent:-.25in;
mso-list:l0 level1 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>2.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]>If
<b>s</b> is a method definition scope (1a) then this method’s frame <b>f</b> is
available: either it is <o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.25in;text-indent:-.25in;
mso-list:l0 level2 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>a.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]>the
frame that invokes the ‘super’ call, or<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.25in;text-indent:-.25in;
mso-list:l0 level2 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>b.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]>captured
by a closure of the block that contains the ‘super’ call<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:.75in;text-indent:-.25in;
mso-list:l0 level1 lfo4'><![if !supportLists]><span style='mso-list:Ignore'>3.<span
style='font:7.0pt "Times New Roman"'> </span></span><![endif]>If
<b>s</b> is a block (1b) then its caller’s frame <b>f</b> is available.<o:p></o:p></p>
<p class=MsoNormal><o:p> </o:p></p>
<p class=MsoNormal>
Use the arguments, self object and method name of the frame <b>f </b>for the
‘super’ invocation.<o:p></o:p></p>
<p class=MsoNormal style='text-indent:.5in'><o:p> </o:p></p>
<p class=MsoNormal style='margin-left:.5in'>Implementation: the method defined
by define_method passes its frame into the block it calls (in BlockParam
object). Super call site traverses lexical hierarchy and finds scope <b>s</b>
and frame <b>f</b>. All information necessary to perform the super call are on <b>f</b>.
For super to work within eval some additional work needs to be done beyond this
shelveset – arguments needs to be distinguishable from other local variables at
run-time. Super in eval will then do dynamic local variable lookup on the
argument names that it finds in the frame <b>f</b>. This change doesn’t
implement any optimizations though they are possible in some cases.<o:p></o:p></p>
<p class=MsoListParagraph><o:p> </o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Adds ProtocolConverterAction custom action.<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>It’s an abstract class for specific protocol
conversions (to_proc, to_s, …)<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>This shelveset adds only ConvertToProcAction
<: ProtocolConverterAction so far, we can replace Protocols by these actions
in future.<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>The protocol used for conversions is:<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;
mso-list:l1 level3 lfo2'><![if !supportLists]><span style='font-family:Symbol'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>if obj.respond_to? :to_xxx <br>
result = obj.to_xxx <br>
if result.subclass_of Xxx then return result<br>
end<br>
errror<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>The action implements an optimization that
avoids 2 dynamic dispatches in case respond_to? is not overridden:<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;
mso-list:l1 level3 lfo2'><![if !supportLists]><span style='font-family:Symbol'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>check class version of object obj in rule test<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;
mso-list:l1 level3 lfo2'><![if !supportLists]><span style='font-family:Symbol'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>obj.class overrides respond_to? => no
optimization (emit 2 dynamic dispatches to respond_to? and to_xxx)<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;
mso-list:l1 level3 lfo2'><![if !supportLists]><span style='font-family:Symbol'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>otherwise: lookup to_xxx, not found => emit
error, found => emit static call to to_xxx<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.5in;text-indent:-.25in;
mso-list:l1 level3 lfo2'><![if !supportLists]><span style='font-family:Symbol'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>check result of to_xxx<o:p></o:p></p>
<p class=MsoListParagraph><o:p> </o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Implements to_proc conversion for & operator using
ConvertToProcAction (fixes bug <span style='color:#202020'>#19714: &
operator should call to_proc)</span><o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Adds #method-definition closure local variable of type
RubyMethodInfo: <o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>Each method definition stores itself into this
variable, which is used inside the method definition. This effectively enables
any library/ops code to access the current method definition.<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>RubyMethodInfo now holds on the method’s
declaring local scope.<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in;text-indent:-.25in;
mso-list:l1 level2 lfo2'><![if !supportLists]><span style='font-family:"Courier New"'><span
style='mso-list:Ignore'>o<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]>Adds DefinitionName to RubyMethodInfo – this is
the name of the method definition. The method itself could be used under different
names (if aliased). Some language operations care only about definition name
(like super call).<o:p></o:p></p>
<p class=MsoListParagraph style='margin-left:1.0in'><o:p> </o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Adds UndefineMethod attribute – it is used on library
classes to undefined methods defined in base classes.<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Fixes GetInstanceMethods to skip undefined method.<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Implements Kernel#block_given?, Module#module_function
methods. Fixes Kernel#private/public/protected methods.<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Replaces VariableExpression and ParameterExpression by
Expression where possible.<o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Simplifies code emitted per method definition. <o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Fixes bug <span style='color:#202020'>#19907:
class_eval not singleton.</span><o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]><span style='color:#202020'>Fixes bug #19672: Logical
operator in method arguments won't parse.</span><o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]><span style='color:#202020'>Fixes bug #19803: rescue
next syntax bug.</span><o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]><span style='color:#202020'>Fixes bug #19892: Proc
cannot be used as Boolean.</span><o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]><span style='color:#202020'>Fixes bug #19706: alias
method resolution bug.</span><o:p></o:p></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l1 level1 lfo2'><![if !supportLists]><span
style='mso-list:Ignore'>-<span style='font:7.0pt "Times New Roman"'>
</span></span><![endif]>Adds more unit tests and refactors unit test driver a
little bit.<o:p></o:p></p>
<p class=MsoNormal><o:p> </o:p></p>
<p class=MsoNormal>Tomas<o:p></o:p></p>
<p class=MsoPlainText><o:p> </o:p></p>
</div>
</body>
</html>