[cfe-commits] r143856 - /cfe/trunk/lib/Driver/Tools.cpp

[David Blaikie]

[+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