<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jun 27, 2011, at 5:14 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 Jun 25, 2011, at 2:55 PM, Zach Wheeler wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">This patch is intended to change <font class="Apple-style-span" face="'courier new', monospace">clang::Token::getName()</font> <font class="Apple-style-span" face="arial, helvetica, sans-serif"> so that it returns </font><font class="Apple-style-span" face="'courier new', monospace">llvm::StringRef</font><font class="Apple-style-span" face="arial, helvetica, sans-serif"> instead of </font><font class="Apple-style-span" face="'courier new', monospace">const char*</font><font class="Apple-style-span" face="arial, helvetica, sans-serif">.</font><div>
<div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><br></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif">This is my first patch (ever), so if I did something I wasn't supposed to, just scream at me and I'll try to fix it. :-)</font></div>
</div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif"><br></font></div><div><font class="Apple-style-span" face="arial, helvetica, sans-serif">This turned out to be an easy place to start; the doxygen reference indicates that this method isn't referenced at all. Clang built fine with these changes.</font></div></blockquote><div><br></div><div>Hi Zach,</div><div><br></div><div>This is a great start, but I'd prefer to not change these: if they aren't called at all, please send in a patch to nuke them :).  Also, since these are just returning constant C strings, a "const char*" is actually good enough.</div></div></div></blockquote><div><br></div><div>Surely all of the callers of getName() would have to do an unnecessary strlen here?  Granted that there aren't any. :)  But if we weren't nuking the calls, I don't see why converting them to return StringRef wouldn't be an improvement.</div><div><br></div><div>John.</div></div></body></html>