7

I need to extend std::basic_string to work over path strings and different operator+:

#include <string>

template <class t_elem, class t_traits, class t_alloc>
class path_basic_string : public std::basic_string<t_elem, t_traits, t_alloc>
{
public:
    using base_type = std::basic_string<t_elem, t_traits, t_alloc>;

    path_basic_string() = default;
    path_basic_string(const path_basic_string & ) = default;
    path_basic_string & operator =(const path_basic_string &) = default;

    path_basic_string(const base_type & r) :
        base_type(r)
    {
    }

    path_basic_string(base_type && r) :
        base_type(std::move(r))
    {
    }
};

using path_string = path_basic_string<char, std::char_traits<char>, std::allocator<char> >;

template <class t_elem, class t_traits, class t_alloc>
inline path_basic_string<t_elem, t_traits, t_alloc> &&
    operator +(
        path_basic_string<t_elem, t_traits, t_alloc> && l,
        std::basic_string<t_elem, t_traits, t_alloc> && r)
{
    std::basic_string<t_elem, t_traits, t_alloc> && l_str = std::move(l);
    std::basic_string<t_elem, t_traits, t_alloc> && r_str = std::move(r);

    const bool has_right = !r_str.empty();
    return std::move(
        path_basic_string<t_elem, t_traits, t_alloc>{
            std::move(std::move(l_str) + (has_right ? "/" : "") + (has_right ? std::move(r_str) : std::move(std::basic_string<t_elem, t_traits, t_alloc>{})))
        });
}

template <class t_elem, class t_traits, class t_alloc>
inline path_basic_string<t_elem, t_traits, t_alloc>
    operator +(
        const path_basic_string<t_elem, t_traits, t_alloc> & l,
        const std::basic_string<t_elem, t_traits, t_alloc> & r)
{
    const std::basic_string<t_elem, t_traits, t_alloc> & l_str = l;

    const bool has_right = !r.empty();
    return path_basic_string<t_elem, t_traits, t_alloc>{
        l_str + (has_right ? "/" : "") + (has_right ? r : std::basic_string<t_elem, t_traits, t_alloc>{})
    };
}

int main()
{
    path_string a;
    std::string b;
    std::string c;
    const path_string test = a + (b + c);

    return 0;
}

At the https://godbolt.org/z/jhcWoh i've got these errors:

x86 MSVC 19 2015 U3:

/opt/compiler-explorer/windows/19.00.24210/include/xlocale(341):
warning C4530: C++ exception handler used, but unwind semantics are
not enabled. Specify /EHsc

<source>(61): error C2666: 'operator +': 3 overloads have similar
conversions

<source>(44): note: could be
'path_basic_string<char,std::char_traits<char>,std::allocator<char>>
operator +<char,std::char_traits<char>,std::allocator<char>>(const
path_basic_string<char,std::char_traits<char>,std::allocator<char>>
&,const
std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&)'

<source>(28): note: or      
'path_basic_string<char,std::char_traits<char>,std::allocator<char>>
&&operator
+<char,std::char_traits<char>,std::allocator<char>>(path_basic_string<char,std::char_traits<char>,std::allocator<char>> &&,std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&&)'

/opt/compiler-explorer/windows/19.00.24210/include/xstring(2310):
note: or      
'std::basic_string<char,std::char_traits<char>,std::allocator<char>>
std::operator
+<char,std::char_traits<char>,std::allocator<char>>(const std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&,const
std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&)'

/opt/compiler-explorer/windows/19.00.24210/include/xstring(2380):
note: or      
'std::basic_string<char,std::char_traits<char>,std::allocator<char>>
std::operator
+<char,std::char_traits<char>,std::allocator<char>>(const std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&,std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&&)'

/opt/compiler-explorer/windows/19.00.24210/include/xstring(2390):
note: or      
'std::basic_string<char,std::char_traits<char>,std::allocator<char>>
std::operator
+<char,std::char_traits<char>,std::allocator<char>>(std::basic_string<char,std::char_traits<char>,std::allocator<char>> &&,const
std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&)'

/opt/compiler-explorer/windows/19.00.24210/include/xstring(2400):
note: or      
'std::basic_string<char,std::char_traits<char>,std::allocator<char>>
std::operator
+<char,std::char_traits<char>,std::allocator<char>>(std::basic_string<char,std::char_traits<char>,std::allocator<char>> &&,std::basic_string<char,std::char_traits<char>,std::allocator<char>>
&&)'

<source>(61): note: while trying to match the argument list
'(path_string,
std::basic_string<char,std::char_traits<char>,std::allocator<char>>)'

<source>(61): note: note: qualification adjustment (const/volatile)
may be causing the ambiguity

Compiler returned: 2

x86-64 gcc 5.4 (with --std=c++11):

source>: In function 'int main()':

<source>:61:40: warning: ISO C++ says that these are ambiguous, even
though the worst conversion for the first is better than the worst
conversion for the second:

     const path_string test = a + (b + c);

                                        ^

<source>:44:5: note: candidate 1: path_basic_string<t_elem, t_traits,
t_alloc> operator+(const path_basic_string<t_elem, t_traits,
t_alloc>&, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&)
[with t_elem = char; t_traits = std::char_traits<char>; t_alloc =
std::allocator<char>]

     operator +(

     ^

In file included from
/opt/compiler-explorer/gcc-5.4.0/include/c++/5.4.0/string:52:0,

                 from <source>:1:

/opt/compiler-explorer/gcc-5.4.0/include/c++/5.4.0/bits/basic_string.h:4854:5:
note: candidate 2: std::__cxx11::basic_string<_CharT, _Traits, _Alloc>
std::operator+(const std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]

     operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,

     ^

Compiler returned: 0

I know at least one workaround for that.

But the hell is what even happened? What in the ridiculousness i have to overload again additionally to avoid that overload collision mess?

Update: Fixed by removing const and single reference from all basic_string kind arguments of all operator+. Seems that works it out.

Andry
  • 2,273
  • 29
  • 28
  • 4
    Types from the standard library are a pain in the arse to inherit from. Have you thought of composition instead? It's a world of bliss. – YSC Nov 05 '18 at 13:11
  • i can fix that just replacing one of the `operator+` argument `const std::basic_string<...> & r` by `std::basic_string<...> r`, but what the hell is that? – Andry Nov 05 '18 at 13:17
  • `(b + c)` is a `std::basic_string<...>&&`. Another overload of `operator+` might be preferred compared with the one you're expecting. I really don't want to read those errors. Composition is my advice. – YSC Nov 05 '18 at 13:19
  • 6
    *"I need to extend `std::basic_string`"* You probably don't, really. `std::basic_string` is not designed to be derived from; attempting to do so is unwise, and rarely ends well. – Igor Tandetnik Nov 05 '18 at 13:29
  • 2
    This isn't at all rigorous, but I always get nervous when there are too many forms of inter-convertibility. In this code, a `path_basic_string` can be converted to a `basic_string` through a derived-to-base conversion, and a `basic_string` can be converted to a `path_basic_string` through the constructor. That's often a recipe for ambiguity, and it's one of the primary reasons that `explicit` was added: to rule out one of the implicit conversions. This often comes up with classes that imitate numeric types with both an `operator int()` and a constructor that takes `int`. – Pete Becker Nov 05 '18 at 13:30
  • @IgorTandetnik I know, but that else can I do if i didn't want to write all these proxy-methods into `std::string`? I just wanted to inherit the functionality of `std::string` to extend it, not to overlap with it. But seems it becomes more and more difficult with a simple inheritance. – Andry Nov 05 '18 at 13:35
  • 4
    If you can upgrade your compilers/code base standard C++17 offers [`std::filesystem::path`](https://en.cppreference.com/w/cpp/filesystem/path). (you could always you the boost version as well if you can't) – NathanOliver Nov 05 '18 at 13:37
  • 1
    Sounds like an XY problem. – Mike Borkland Nov 05 '18 at 13:42
  • @NathanOliver In mine case i have only `C++11` for the back compatability. – Andry Nov 05 '18 at 13:51
  • 2
    @Andry *I just wanted to inherit the functionality of std::string to extend it* -- Use `std::string` as-is. Once you see you need to "extend" it, create or use a string utility class / namespace that provides this extension. Probably you will wind up seeing that the extension(s) you want are few and far between. – PaulMcKenzie Nov 05 '18 at 13:55
  • @PaulMcKenzie This may be not the case because i need a class to put into another methods in further overloading. In case of utility class / namespace it would be a kind of mess to force a user to use it, instead of just use an additional functionality over a function parameter. – Andry Nov 05 '18 at 14:02
  • 1
    Format error messages as quoted code; add 5 or 6 spaces after the >. – Yakk - Adam Nevraumont Nov 05 '18 at 14:03
  • 2
    Even if you're not using C++17 today, it helps to be API-compatible so you can just switch implementations with a single `using path = `. And that means `operator+=` does not append a `/`, there's `operator/=` for that. – MSalters Nov 05 '18 at 15:08

1 Answers1

2

First things first, use move-from-value instead of const& and && overloads.

path_basic_string(base_type r) :
    base_type(std::move(r))
{
}

and get rid of base_type const& ctor.

Second, make that ctor explicit:

explicit path_basic_string(base_type r) :
    base_type(std::move(r))
{
}

as a path is a different thing than a string.

Third, clean up your template operator+ and make it an ADL "Koenig" operator that takes its left hand side by value. Oh, and don't return anything by rvalue reference, that is toxic.

friend path_basic_string
    operator +(
        path_basic_string l,
        base_type const& r)
{
  base_type& l_str = l;
  if (!r.empty())
    l = path_basic_string( std::move(l_str) + "/" + r );
  return l;
}

and get rid of all that noise.

Next, inherit ctors from base_type.

Finally, implement appending using += and make the operations symmetric:

template <class t_elem, class t_traits, class t_alloc>
class path_basic_string : public std::basic_string<t_elem, t_traits, t_alloc>
{
public:
    using base_type = std::basic_string<t_elem, t_traits, t_alloc>;

    path_basic_string() = default;
    path_basic_string(const path_basic_string & ) = default;
    path_basic_string & operator =(const path_basic_string &) = default;

    using base_type::base_type;

    explicit path_basic_string(base_type r) :
        base_type(std::move(r))
    {
    }
    path_basic_string& operator+= ( base_type const& rhs ) & {
      if (!rhs.empty())
      {
        base_type& self = *this;
        self += '/';
        self += rhs;
      }
      return *this;
    }
    friend path_basic_string operator+(
            base_type l,
            base_type const& r
    )
    {
      path_basic_string l_path(std::move(l));
      l+=r;
      return l;
    }
};

operator+ here is fancy as it is only findable via ADL, yet it actually operates on the base type of the class.

This means that at least one of the arguments must be an instance of this type (or have an instance of this type as a template argument) in order for it to be found.

Then conversion-to-base occurs if required.

I take LHS by value, because moving a string is cheap-to-free, and we need a string for output. By taking LHS by value and using its buffer (after moving it) for the return value, we get efficient chained addition:

a+b+c+d+e

becomes

(a+b)+c+d+e

now the return value of a+b (a prvalue) is then used as the lhs argument of (a+b)+c.

This recycling of the buffer continues; only one buffer is created (from the first +), and it is then moved, resized (hopefully efficiently) and reused for the rest of the expression.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Isn't these `friend` operators are toxic too? – Andry Nov 05 '18 at 14:21
  • @Andry No, koenig operators are a good plan. They are non-template operators on a template class, and have many wonderful properties. If you want you can have them "bounch" to a helper function that doesn't have that level of permission. Also I'd do `+=` and implmeent `+` in terms of `+=` myself,. – Yakk - Adam Nevraumont Nov 05 '18 at 14:24
  • @YakkAdamNevraumont But they are asymmetric, i want them to be symmetric: `basic_string + path_string` behaves the same as `path_string + basic_string`. Is that make sense? – Andry Nov 05 '18 at 14:27
  • @YakkAdamNevraumont Why u don't use `basic_string` by value in operators? – Andry Nov 05 '18 at 14:51
  • @Andry Actually, there appears to be an MSVC bug with `operator+` overload resolution; the same set of overloads not in an `operator+` generate 0 ambiguity, but in an operator does. I simplified and worked around that issue. I don't, because by-value on left gives you efficient chain-addition, and by-value/rvalue on right usually gives you nothing useful. – Yakk - Adam Nevraumont Nov 05 '18 at 14:57
  • @YakkAdamNevraumont This is why i prefer to use outside operators instead of friend ones. The msvc sometimes complains on them which gives me no choice other than to put them outside. – Andry Nov 05 '18 at 15:00
  • 1
    @Andry regardless, what is posted above is small, efficient and solves your problem now. Only one `operator+`. I described why and how it works as well. – Yakk - Adam Nevraumont Nov 05 '18 at 15:02
  • @YakkAdamNevraumont Thanks for the effort, but anyway, all of these have to be retested in real environment and the choice to use it has come from the responce of the actual set of compilers, tool sets and of cause the back compatability. – Andry Nov 05 '18 at 15:07