<HTML><BODY style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space; "><BR><DIV><DIV>On Oct 4, 2007, at 2:48 PM, Chris Lattner wrote:</DIV><BR class="Apple-interchange-newline"><BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">On Oct 4, 2007, at 11:37 AM, Steve Naroff wrote:</DIV> <BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">On Oct 4, 2007, at 11:01 AM, Chris Lattner wrote:</DIV> <BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">On Sep 27, 2007, at 11:57 AM, Fariborz Jahanian wrote:</DIV> <BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+<SPAN class="Apple-converted-space"> </SPAN>for (int j = 0; j < IDecl->getNumInsMethods(); j++)</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+<SPAN class="Apple-converted-space"> </SPAN>if (!Map.count(methods[j]->getSelector())) {</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+<SPAN class="Apple-converted-space"> </SPAN>llvm::SmallString<128> buf;</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+<SPAN class="Apple-converted-space"> </SPAN>Diag(methods[j]->getLocation(), diag::warn_undef_method_impl,</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+ <SPAN class="Apple-converted-space"> </SPAN>methods[j]->getSelector()->getName(buf));</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+<SPAN class="Apple-converted-space"> </SPAN>}</DIV> </BLOCKQUOTE><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">This seems awkward: I think it would make sense to add a getNameAsStr</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">() method which returns an std::string directly (or have a getName()</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">which takes no argument).<SPAN class="Apple-converted-space"> </SPAN>With the current code, an std::string is</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">being transparently constructed anyway, so there should be no perf</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">penalty.</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV> </BLOCKQUOTE><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">I don't understand why this is awkward, since this is similar to Type::getAsStringInternal(buf). Aren't they solving the same problem? curious.</DIV> </BLOCKQUOTE><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Ah, I see the inspiration.<SPAN class="Apple-converted-space"> </SPAN>The difference here is that Type::getAsStringInternal needs a buffer to mutate internally before it returns it.<SPAN class="Apple-converted-space"> </SPAN>It is adding prefixes and suffixes onto it.<SPAN class="Apple-converted-space"> </SPAN>You're right that it should have a form that returns an std::string for use in diagnostics.</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV> <BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">btw...what I find more awkward about the code snippit is...</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV> <BLOCKQUOTE type="cite"><BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">+<SPAN class="Apple-converted-space"> </SPAN>if (!Map.count(methods[j]->getSelector())) {</DIV> </BLOCKQUOTE></BLOCKQUOTE><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Since I am reading the code in context, I can imagine that "count" is being used as a "lookup" function.</DIV> </BLOCKQUOTE><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Hey, I totally agree with you here. Unfortunately "count" is an idiom that comes from the STL.<SPAN class="Apple-converted-space"> </SPAN>:(<SPAN class="Apple-converted-space"> </SPAN>I assume that it is for generality across multimap and multisets where a key can be in the container more than once.</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV> <BLOCKQUOTE type="cite"><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">Nevertheless, in isolation, the preceding expression doesn't read well or make sense (I discussed this with Fariborz when I reviewed his patch...he said Chris said this was the most efficient:-)</DIV> </BLOCKQUOTE><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">The getName() expression wouldn't be as efficient?</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV></BLOCKQUOTE><DIV><BR class="khtml-block-placeholder"></DIV>My previous comment applied to the Map.count idiom (not getName()). I have no problem with the efficiency of getName()...obviously:-) </DIV><DIV><BR class="khtml-block-placeholder"></DIV><DIV>As far as getName() is concerned, I am concerned about lifetime. By passing a buffer in, it is clear the client owns the buffer. If getName() returns a string buffer, the ownership is unclear. I don't have a strong opinion on this...I just want to point out my (minor) issue/concern. Clearly, not having to pass in a buffer is more convenient...</DIV><DIV><BR class="khtml-block-placeholder"></DIV><DIV>snaroff</DIV><DIV><BR class="khtml-block-placeholder"></DIV><DIV><BLOCKQUOTE type="cite"><FONT class="Apple-style-span" color="#000000"><BR class="khtml-block-placeholder"></FONT><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; ">-Chris</DIV><DIV style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; min-height: 14px; "><BR></DIV> </BLOCKQUOTE></DIV><BR></BODY></HTML>