Opened 7 years ago

Closed 6 years ago

#11684 closed Feature Requests (fixed)

boost::string_ref lack of move constructor prevents efficient use in some stl containers

Reported by: banks@… Owned by: No-Maintainer
Milestone: To Be Determined Component: utility
Version: Boost 1.59.0 Severity: Optimization
Keywords: Cc:

Description

Not quite sure if this is a bug or a feature request given the statement in the docs:

"string_ref does not define a move constructor nor a move-assignment operator because copying a string_ref is just a cheap as moving one."

While that line makes total sense on it's own, it does have some undesirable (for me at least) consequences.

It means that and class that may want to have string_refs as members also can't be trivially move constructible even if their other members are expensive to copy.

For custom classes that can be worked around by defining a move constructor that copies the string_ref members and moves the rest. But it's a limitation that can't be easily worked around for things like std::pair or std::tuple since even if you try specialising those templates, you can't redefine the explicitly defaulted move constructor.

This is especially an issue for using string_ref as a key or value type in an associative container - in this case the std::pair type MUST be copied during insertion. If the other type in the pair is expensive to copy then string_refs lack of move construction becomes an annoying performance barrier.

Workarounds exist so it's hardly high priority, but it does reduce the reusability of string_ref and cause there to be cases where an alternative implementation might be used for essentially the same purpose to work around this issue. the cost of adding move construction seem low to me even if it might result in uninformed users using it spuriously under the assumption it will provide some performance increase over copying.

To be clear the request is: please consider adding move constructor/assignment to boost::string_ref.

Change History (6)

comment:1 by anonymous, 7 years ago

Summary: boost::string_ref lack of move constructor prevents efficient use in std::mapboost::string_ref lack of move constructor prevents efficient use in some stl containers

comment:2 by Marshall Clow, 6 years ago

I'm confused. I tried this:

struct S { std::string_view sv; };

int main () {
    static_assert( std::is_trivially_move_constructible<std::string_view>::value, "" );
    static_assert( std::is_trivially_move_constructible<S>::value, "" );
    static_assert( std::is_trivially_constructible<std::string_view, std::string_view&&>::value, "" );
    }

and it compiled w/o error. (clang, libc++)

comment:3 by Marshall Clow, 6 years ago

My bad. Just tried again with boost::string_ref, and I see what's up. Looking into it...

comment:4 by Marshall Clow, 6 years ago

I don't think that adding move ctor/assignment to string_ref will help. To be "trivial', they have to be defaulted. If the copy ctor/assignment is defaulted, that's sufficient.

comment:5 by Marshall Clow, 6 years ago

Commit c56dd13592daa332a74b90aeca8724c5b3cc3d5d should take care of this.

comment:6 by Marshall Clow, 6 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.