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

Ahmed Charles 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. :)

-- 
Ahmed Charles




More information about the cfe-commits mailing list