On Mon, Oct 1, 2012 at 1:44 PM, Dmitri Gribenko <span dir="ltr"><<a href="mailto:gribozavr@gmail.com" target="_blank">gribozavr@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Mon, Oct 1, 2012 at 2:34 PM, Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br>
> On Sat, Sep 29, 2012 at 1:40 PM, Dmitri Gribenko <<a href="mailto:gribozavr@gmail.com">gribozavr@gmail.com</a>><br>>> +  /// return the last one defined.<br>
>> +  StringRef getLastMacroWithSpelling(SourceLocation Loc,<br>
>> +                                     ArrayRef<TokenValue> Tokens) const;<br>><br>
> Isn't it more convenient to have a default spelling as a parameter of this<br>
> method so that callers don't have to check if the result is empty?<br>
<br>
</div>It seemed to me to violate the principle of single responsibility.<br></blockquote><div><br></div><div>I think the responsibility of this method (may require name change) can be "give me the most proper spelling for this snippet at this code location". So providing default value seems to be a good thing to me.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
>><br>
>> +          SmallString<64> TextToInsert(AnnotationSpelling);<br>
>> +          TextToInsert += "; ";<br>
><br>
> Over-optimization? ;)<br>
<br>
</div>Just the usual thing to use SmallString to avoid dynamic allocations.<br></blockquote><div><br></div><div>I know what it does, but I'm not sure that this makes sense in this particular code location. It's not going to be executed frequently, and this particular optimization is not going to speed up this code much. I would prefer to leave it as is, but it's not a big deal.</div>
</div><div><br></div>-- <br>
<div>Regards,</div><div>Alex</div>