Re: [boost] Constructing string_ref from rvalue string

Subject: Re: [boost] Constructing string_ref from rvalue string
From: Gavin Lambert (gavinl_at_[hidden])
Date: 2017-03-20 02:42:43

On 17/03/2017 20:29, Simon Sasburg via Boost wrote:
> I've just tested our codebase with the 1.64.0 master snapshot, and i've
> encountered an issue which i've tracked down to this commit:
>
> https://github.com/boostorg/utility/commit/9960d9f395b79ee860e39064cd4696
> 1f76d2cb55
>
> Basically this removes the constructor which allows constructing a
> boost::string_ref from an rvalue std::string. I am wondering if this is
> really that dangerous. In our codebase we almost exlusively use
> boost::string_ref as function parameters, with the understanding that
> passed string_refs parameters are only valid within the function body. (so
> you need to copy it into an std::string if you need to access it afterwards)
> With these constraints i think it is OK to construct a string_ref from an
> rvalue std::string because the temporary should live long enough. (but
> please correct me if i'm wrong here)

If you are calling a function that returns a std::string (thus you have 
an rvalue temporary) and you are immediately wrapping this in a 
string_ref (as another temporary) and then passing this as a function 
parameter, AFAIK the compiler is within its rights to delete the 
temporary std::string prior to the call as it is not the actual parameter.

(To put it more generally: only the temporaries that are the actual 
parameters are "safe" -- any temporaries used to construct those 
temporaries might be deleted prior to the call.)

Although you might get away with it sometimes (perhaps even most of the 
time) as it probably wouldn't do so without aggressive optimisation -- 
but you shouldn't rely on this; it's unsafe usage and subject to the 
whims of the compiler.

The safe version is to store the returned std::string as an lvalue (eg. 
a local variable) prior to the function call that constructs the 
temporary rvalue string_refs.

> Is the safety (?) added by this commit worth it if it breaks working code?

Can you post an example of such working code that's broken by this 
change?  (Assuming that it's not covered by the above case.)