<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Sep 10, 2010, at 3:02 PM, Chris Lattner wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Sep 10, 2010, at 2:59 PM, Douglas Gregor wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><br>On Sep 10, 2010, at 2:15 PM, Sebastian Redl wrote:<br><br><blockquote type="cite">Author: cornedbee<br></blockquote><blockquote type="cite">Date: Fri Sep 10 16:15:56 2010<br></blockquote><blockquote type="cite">New Revision: 113634<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=113634&view=rev">http://llvm.org/viewvc/llvm-project?rev=113634&view=rev</a><br></blockquote><blockquote type="cite">Log:<br></blockquote><blockquote type="cite">Follow Ted's example and hide CXXNoexceptExpr's setters from all but serialization.<br></blockquote><br>I've actually been leaning toward removing the setters entirely, then letting serialization poke at the fields directly.<br></div></blockquote></div><br><div>I completely agree. In the Expr classes, I think that all the ivars should be public. Ones that are fixed at construction time (like the ivars in types) should just be marked const:</div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "><span style="color: #bb2d9d">class</span> FloatingLiteral : <span style="color: #bb2d9d">public</span> Expr {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; ">public:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> APFloatStorage Num;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> <span style="color: #bb2d9d">bool</span> IsExact : <span style="color: #2c2ecf">1</span>;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> SourceLocation Loc;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; min-height: 14px; ">private:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> FloatingLiteral(ASTContext &C, <span style="color: #bb2d9d">const</span> llvm::APFloat &V, <span style="color: #bb2d9d">bool</span> isexact,</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> QualType Type, SourceLocation L)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> : Expr(FloatingLiteralClass, Type, <span style="color: #bb2d9d">false</span>, <span style="color: #bb2d9d">false</span>),</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> IsExact(isexact), Loc(L) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> setValue(C, V);</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; "> }</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; min-height: 14px; ">...</div></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; min-height: 14px; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 13px/normal Inconsolata; min-height: 14px; ">I don't see any reason to have tons of ivars that have trivial getters and setters, it's just a lot of redundant code.</div></div></blockquote><br></div><div>Please don't. Having explicit getters makes it easier for us to tweak the internal representation (to save space, or keep track of more information, or whatever). In general, we shouldn't have setters [*], and serialization should just poke at the fields directly.</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><br><div>[*] Most of these are my fault, since we're taking blame ;)</div></body></html>