[utility] string_ref construction from rvalue reference to string
Hello, boost. Are the any reasonable cases when construction boost::string_ref from std::string && is desirable? I mean that usage string_ref in the following examples is totally wrong, but not for compiler: using std::string; using boost::string_ref; // 1 { auto ref = string_ref(string("abacaba") + string("upyachka")); std::cout << ref << std::endl; } // 2 { string very_long(1000000, 'a'); auto ref = string_ref(very_long + "b"); std::cout << ref << std::endl; } In practice it gets worse, because the first example example works (gcc 4.9.1) thanks to optimizations with short literals, I suspect, and the second example leads to segmentation fault, as expected. 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<charT, traits, Allocator> &&) = delete; I cannot understant does it prevent any correct usage of string_ref? -- Andrey Davydov
Андрей Давыдов 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<charT, traits, Allocator> &&) = 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.
Nate
On Aug 1, 2014, at 1:38 PM, Nathan Crookston <nathan.crookston@gmail.com> 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<charT, traits, Allocator> &&) = 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
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<charT, traits, Allocator> &&) = 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. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
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<charT, traits, Allocator> &&) = 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 Sat, Aug 2, 2014 at 2:34 AM, Agustín K-ballo Bergé <kaballo86@hotmail.com
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. On Sat, Aug 2, 2014 at 2:46 AM, Nathan Crookston <nathan.crookston@gmail.com
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())`. 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`. 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. On Sat, Aug 2, 2014 at 2:24 AM, Marshall Clow <mclow.lists@gmail.com> wrote:
It would mean that string_ref will no longer compile under C++03.
Of course, I propose to hide it under corrsponding ifdefs. Something like this: #ifndef BOOST_NO_CXX11_RVALUE_REFERENCES #ifndef BOOST_NO_CXX11_DELETED_FUNCTIONS template<typename Allocator> basic_string_ref(std::basic_string<charT, traits, Allocator> &&) = delete; #else private: template<typename Allocator> basic_string_ref(std::basic_string<charT, traits, Allocator> &&); public: #endif template<typename Allocator> basic_string_ref(const reference_wrapper< const std::basic_string< charT, traits, Allocator> > rw) : ptr_(rw.get().data()) , len_(rw.get().length()) {} #endif -- Andrey
On Sunday 03 August 2014 16:27:13 Andrey Davydov wrote:
On Sat, Aug 2, 2014 at 2:46 AM, Nathan Crookston <nathan.crookston@gmail.com
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 Sun, Aug 3, 2014 at 4:55 PM, Andrey Semashev <andrey.semashev@gmail.com> wrote:
boost::reference_wrapper will not construct from rvalues as of 1.56. std::reference_wrapper already doesn't.
OK, it's possible to write custom function template<class charT, class traits, class Allocator> basic_string_ref<charT, traits> unsafe_make_ref(std::basic_string<charT, traits, Allocator> && str) { return { str }; } and use it. 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.
I'm not sure that one extra call [`print(unsafe_make_ref(getStr()))` instead of `print(getStr())`] is too high usability price for the safety in common case. Of course, it's only IMHO. -- Andrey
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é <kaballo86@hotmail.com
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
participants (6)
-
Agustín K-ballo Bergé -
Andrey Davydov -
Andrey Semashev -
Marshall Clow -
Nathan Crookston -
Андрей Давыдов