[cfe-commits] r143856 - /cfe/trunk/lib/Driver/Tools.cpp
dblaikie at gmail.com
Sun Nov 6 08:15:34 PST 2011
[+Ahmed Charles, author of the patch]
>> > - StringRef ArgString = A->getAsString(Args);
>> > + const std::string &ArgString = A->getAsString(Args);
> Since getAsString returns a std::string and not a reference, I think the
> code would be more explicit if you were to shed the & as it just
> introduces a binding to a temporary.
Yeah, I was on the fence about that too. Do you think that's better
just for clarity (it's not clear to me whether you're saying it's
incorrect/invalid, which it isn't - the reference bound to a temporary
extends the lifetime of that temporary to the scope of the reference),
I agree this feature isn't exactly intuitive/obvious.
I believe (to paraphrase) Ahmed's preference for this solution was
that it's correct no matter whether getAsString returns by value or
const ref, and more efficient in the latter case, so a useful generic
habit to get into unless you explicitly need a copy for some reason
(mutation either of the local or the original string).
I'm (& I'm sure he is too) certainly open to discussion on the issue.
More information about the cfe-commits