On Aug 1, 2014, at 1:38 PM, Nathan Crookston wrote:

Андрей Давыдов wrote:

...

Those errors can be caught at compile time if class "basic_string_ref" defintion would contain lines like following:

template<typename Allocator> basic_string_ref(std::basic_string &&) = delete;

I cannot understant does it prevent any correct usage of string_ref?

+1. I can't think of a good reason to allow this.

It would mean that string_ref will no longer compile under C++03. [ But it can certainly be done for C++11 and later ] — Marshall

Hi Agustín, Agustín K-ballo Bergé wrote:

On 31/07/2014 04:53 a.m., Андрей Давыдов wrote:

...

Those errors can be caught at compile time if class "basic_string_ref" defintion would contain lines like following:

template<typename Allocator> basic_string_ref(std::basic_string &&) = delete;

I cannot understant does it prevent any correct usage of string_ref?

I don't think this is such a good idea. Unless I'm mistaken, it would break the typical case of calling a function taking a `string_ref` with a temporary.

A `string_ref` does not own the contents it represents, and the only way to use it correctly is by guaranteeing that the lifetime of the contents extends past that of the reference. You wouldn't generally use `string_ref`s as automatic variables or members.

I suppose that would break cases like: std::string getStr(); print(std::string_ref sr) { std::cout << sr << std::endl; } print(getStr()); Causing people to have to write print(getStr().c_str()). There doesn't seem to be a good, compile-time way to catch misuse. Your suggestion on avoiding using string_ref as a class member seems good. Nate

On Sunday 03 August 2014 16:27:13 Andrey Davydov wrote:

On Sat, Aug 2, 2014 at 2:46 AM, Nathan Crookston

...

wrote:

I suppose that would break cases like:

std::string getStr(); print(std::string_ref sr) { std::cout << sr << std::endl; }

print(getStr());

Causing people to have to write print(getStr().c_str()).

It's possible to add `string_ref(reference_wrapper<const std::string>)` constructor to allow people write `print(boost::cref(getStr()))` instead of `print(getStr().c_str())`.

boost::reference_wrapper will not construct from rvalues as of 1.56. std::reference_wrapper already doesn't. Even then, IMHO string_ref should not require these tricks, it should work transparently. I'm ok with leaving it vulnerable to incorrect use if it's not possible to protect it without sacrificing usability.

On 03/08/2014 09:27 a.m., Andrey Davydov wrote:

On Sat, Aug 2, 2014 at 2:34 AM, Agustín K-ballo Bergé

...

wrote:

...

You wouldn't generally use `string_ref`s as automatic variables or members.

I can't confirm or refute this statement using code of Boost, because `boost::string_ref` has been used a little so far. But results of "grep StringRef" inside LLVM code show that StringRef's are used often as automatic variables. And in addition to explicitly defined class members there are another dangerous `string_ref` use cases, namely lambda capturing value and `bind` argument.

LLVM has some memory management that could be considered extreme, and it does things to avoid even a single allocation (for good reason). Nevertheless, you'll note that I said "wouldn't generally", and not "can't", "won't" or "dangerous". There's nothing specially dangerous about `string_ref` to temporaries, or as members. In order to use `string_ref`, even for the simplest of use cases, you have to guarantee that the referred content will outlive it. If you use a `string_ref` as a function parameter, the language will guarantee that for you, regardless of the value category of the argument. Otherwise, it's "dangerous" as in you have think what you are doing. I took your idea a step further, and implemented your proposed change to `StringRef`. I got hundreds of compilation errors. Most of them stemmed from construction of `StringRef` from temporaries, not explicitly as in your example, but as arguments to functions. There where other sources of errors, like comparisons between `StringRef` and a temporary.

It's possible to add `string_ref(reference_wrapper<const std::string>)` constructor to allow people write `print(boost::cref(getStr()))` instead of `print(getStr().c_str())`. The first version is more efficient and less ugly IMO. I want to emphasize, that it should be `boost::cref` not `std:: cref`, because `std::cref(std::string &&) = deleted`.

Not as of 1.56.

It seems to me, that prohibition of constructing `reference_wrapper` from temporary in C++11 is one more argument in favor of my proposition to prohibit `string_ref` construction from temporary string.

A `reference_wrapper` is a wrapper for an lvalue reference. The motivation to prohibit binding to an rvalue is a different one. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com