<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 15, 2007, at 10:21 AM, Chris Lattner wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Nov 15, 2007, at 9:33 AM, Fariborz Jahanian wrote:<br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">--- cfe/trunk/include/clang/AST/Expr.h (original)<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">+++ cfe/trunk/include/clang/AST/Expr.h Wed Nov 14 20:58:25 2007<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">@@ -1256,18 +1256,23 @@<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> class ObjcIvarDecl *D;<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> SourceLocation Loc;<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"> Expr *Base;<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">- bool IsArrow; // True if this is "X->F", false if this is<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">"X.F".<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">+ bool IsArrow:1; // True if this is "X->F", false if this is<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">"X.F".<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">+ bool IsFreeIvar:1; // True if ivar reference has no base (self<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">assumed).<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">What is IsFreeIvar used for? Shouldn't references to fields of the<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">current objects just use a declrefexpr?<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">It is ivar reference without the base expressions ('self') is assumed. This is to 'attach' the base during rewrite and also for pretty printing to come out exactly as user wrote the code. Use of declrefexpr probably masks what user wrote.<br></blockquote><br>I assume that this is similar to a C++ example like this:<br><br>class foo {<br> int x;<br><br> int bar() { return x; }<br>};<br><br>Why is it more natural to refer to "x" as a field reference than as a declref of the FieldDecl? I know that GCC explicitly does lowering when is parses (making this a field reference off the hidden this/self parameter), but that doesn't seem like the right choice for our ASTs.<br><br>Thoughts?<br></blockquote></div><br><div>Chris,</div><div><br class="webkit-block-placeholder"></div><div>It's a little more complicated than the above example. Consider this pattern from "Sketch"...with the exception of the "copy" variable, all other variable references in this example are represented by "ObjCIvarRefExpr" (_bounds, _isDrawingFill, etc.).</div><div><br class="webkit-block-placeholder"></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">- (<span style="color: #aa0d91">id</span>)copyWithZone:(<span style="color: #5c2699">NSZone</span> *)zone {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; min-height: 14px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span class="Apple-style-span" style="color: rgb(0, 0, 0); "> <span style="color: #3f6e74">SKTGraphic</span> *copy = [[[<span style="color: #aa0d91">self</span> <span style="color: #2e0d6e">class</span>] <span style="color: #2e0d6e">alloc</span>] <span style="color: #2e0d6e">init</span>];</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> copy-><span style="color: #5c2699">_bounds</span> = <span style="color: #3f6e74">_bounds</span>;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> copy-></span>_isDrawingFill<span style="color: #000000"> = </span>_isDrawingFill<span style="color: #000000">;</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> copy-></span>_fillColor<span style="color: #000000"> = [</span>_fillColor<span style="color: #000000"> </span><span style="color: #2e0d6e">copy</span><span style="color: #000000">];</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> copy-></span>_isDrawingStroke<span style="color: #000000"> = </span>_isDrawingStroke<span style="color: #000000">;</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> copy-></span>_strokeColor<span style="color: #000000"> = [</span>_strokeColor<span style="color: #000000"> </span><span style="color: #2e0d6e">copy</span><span style="color: #000000">];</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> copy-></span>_strokeWidth<span style="color: #000000"> = </span>_strokeWidth<span style="color: #000000">;</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">return</span> copy;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">}</div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br class="webkit-block-placeholder"></span></font></div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><span class="Apple-style-span" style="font-family: Helvetica; font-size: 12px; "><div>We never "lower" the ivar references. The AST reflects the code above. For example, there are no synthesized "MemberExpr's" for the ivar references on the right hand side of the assignment expressions above. ObjCIvarRefExpr represents both patterns. The arrow for the left-hand side ivar references is really part of the ObjC syntax (since the type of "copy" is an interface, not a structure).</div><div><br class="webkit-block-placeholder"></div><div>That said, I still think what we have can be improved. From my perspective, ObjCIvarRefExpr could be improved to represent the "common" case more naturally, where we don't need an "Expr *" (since "self" is assumed). This would both save space and remove improve unnecessary code from Sema::ActOnIdentifierExpr(). Here's what I am talking about...</div><div><br class="webkit-block-placeholder"></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">if</span> (CurMethodDecl) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #3f6e74">ObjcInterfaceDecl</span> *<span style="color: #3f6e74">IFace</span> = CurMethodDecl->getClassInterface();</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> </span>ObjcInterfaceDecl<span style="color: #000000"> *</span>clsDeclared<span style="color: #000000">;</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">if</span> (<span style="color: #3f6e74">ObjcIvarDecl</span> *<span style="color: #3f6e74">IV</span> = <span style="color: #3f6e74">IFace</span>->lookupInstanceVariable(&<span style="color: #3f6e74">II</span>, <span style="color: #3f6e74">clsDeclared</span>)) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #3f6e74">IdentifierInfo</span> &<span style="color: #3f6e74">II</span> = <span style="color: #3f6e74">Context</span>.Idents.<span style="color: #2e0d6e">get</span>(<span style="color: #c41a16">"self"</span>); // unnecessary.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #3f6e74">ExprResult</span> SelfExpr = <span style="color: #26474b">ActOnIdentifierExpr</span>(<span style="color: #3f6e74">S</span>, <span style="color: #3f6e74">Loc</span>, <span style="color: #3f6e74">II</span>, <span style="color: #aa0d91">false</span>); // ditto.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">return</span> <span style="color: #aa0d91">new</span> <span style="color: #3f6e74">ObjCIvarRefExpr</span>(<span style="color: #3f6e74">IV</span>, <span style="color: #3f6e74">IV</span>->getType(), <span style="color: #3f6e74">Loc</span>, </div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">static_cast</span><<span style="color: #3f6e74">Expr</span>*>(SelfExpr.<span style="color: #3f6e74">Val</span>), <span style="color: #aa0d91">true</span>, <span style="color: #aa0d91">true</span>);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> }</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> }</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><br class="webkit-block-placeholder"></div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><span class="Apple-style-span" style="font-family: Helvetica; font-size: 12px; "><div>...would become.... </div><div><br class="webkit-block-placeholder"></div></span></span></font></div>
</div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: rgb(170, 13, 145); ">if</span> (CurMethodDecl) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: rgb(63, 110, 116); ">ObjcInterfaceDecl</span> *<span style="color: rgb(63, 110, 116); ">IFace</span> = CurMethodDecl->getClassInterface();</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: rgb(0, 0, 0); "> </span>ObjcInterfaceDecl<span style="color: rgb(0, 0, 0); "> *</span>clsDeclared<span style="color: rgb(0, 0, 0); ">;</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: rgb(170, 13, 145); ">if</span> (<span style="color: rgb(63, 110, 116); ">ObjcIvarDecl</span> *<span style="color: rgb(63, 110, 116); ">IV</span> = <span style="color: rgb(63, 110, 116); ">IFace</span>->lookupInstanceVariable(&<span style="color: rgb(63, 110, 116); ">II</span>, <span style="color: rgb(63, 110, 116); ">clsDeclared</span>))</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: rgb(170, 13, 145); ">return</span> <span style="color: rgb(170, 13, 145); ">new</span> <span style="color: rgb(63, 110, 116); ">ObjCIvarRefExpr</span>(<span style="color: rgb(63, 110, 116); ">IV</span>, <span style="color: rgb(63, 110, 116); ">IV</span>->getType(), <span style="color: rgb(63, 110, 116); ">Loc);</span> </div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> }</div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br class="webkit-block-placeholder"></span></font></div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><span class="Apple-style-span" style="font-family: Helvetica; font-size: 12px; "><div>For now, I suggest we make the change above and have "Base" be null for normal ivar references. As extra credit, we could factor this out into two classes for each of the above cases.</div><div><br class="webkit-block-placeholder"></div><div>snaroff</div><div><br class="webkit-block-placeholder"></div></span></span></font></div></div></span></span></font></div>
</div></body></html>