[cfe-commits] r143856 - /cfe/trunk/lib/Driver/Tools.cpp
ahmedcharles at gmail.com
Sun Nov 6 09:33:44 PST 2011
On Sun, Nov 6, 2011 at 8:15 AM, David Blaikie <dblaikie at gmail.com> wrote:
> [+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.
> - David
That sums up my opinion, yes. :)
More information about the cfe-commits