On Mon, Oct 1, 2012 at 1:44 PM, Dmitri Gribenko <[email protected]> wrote:
> On Mon, Oct 1, 2012 at 2:34 PM, Alexander Kornienko <[email protected]> > wrote: > > On Sat, Sep 29, 2012 at 1:40 PM, Dmitri Gribenko <[email protected]> > >> + /// return the last one defined. > >> + StringRef getLastMacroWithSpelling(SourceLocation Loc, > >> + ArrayRef<TokenValue> Tokens) > const; > > > > Isn't it more convenient to have a default spelling as a parameter of > this > > method so that callers don't have to check if the result is empty? > > It seemed to me to violate the principle of single responsibility. > 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. >> > >> + SmallString<64> TextToInsert(AnnotationSpelling); > >> + TextToInsert += "; "; > > > > Over-optimization? ;) > > Just the usual thing to use SmallString to avoid dynamic allocations. > 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. -- Regards, Alex
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
