url: const_string is not compatible with string_view

const_string::
const_string(
    const_string const& other) noexcept
    : string_view()
{
    if (is_small(other.size()))
    {
        // other is small,
        // nothing to release
        std::memcpy( data_.buf_,
            other.data_.buf_, other.size());
        static_cast<string_view&>(*this) =
            string_view(data_.buf_, other.size());
        return;
    }
    data_.p_ = other.data_.p_;
    ++data_.p_->refs;
    static_cast<string_view&>(
        *this) = string_view(other);
}

const_string inherits from boost::string_view and violates Liscov substitution principle. It is not possible to use const_string exactly like boost::string_view because in string_view data() always point to external memory address (other object), while in const_string may point to own member in which will be freed in destructor.

Following statements are true for boost::string_view but they are false for const_string: std::string text = “test”;

boost::string_view(text).data() == text.data()
boost::string_view(text).data() == boost::string_view(text).data()

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 39 (38 by maintainers)

Commits related to this issue

replace const_string fix #235, fix #223, fix #168, fix #164, fix #163, fix #159 — committed to alandefreitas/url by alandefreitas 2 years ago
replace const_string fix #235, fix #223, fix #168, fix #164, fix #163, fix #159 — committed to alandefreitas/url by alandefreitas 2 years ago
use pct_encoded_view fix #235, fix #223, fix #168, fix #164, fix #163, fix #159 — committed to alandefreitas/url by alandefreitas 2 years ago

Most upvoted comments

We had a parallel discussion about this. It might be that this issue is not really a big problem. I’ll replicate some of the most relevant comments that came up there:

I think I have added an unnecessary confusion in this Issue report. I thought that it is about “every problem with const_string”, but now I see it is only about the problem of:

boost::core::string_view(text).data() == text.data()

The problem boils down to what we consider to be the contract of boost::core::string_view (which tries to mimic std::string_view). The contract has never been specified formally enough, so to some extent it is subject to interpretation; and so is this issue.

My understanding of the string_view’s contract is that:

The object of this type can be cheaply copied around (because it is just a pointer and the int), and both sides agree about this.
Taking a substring in any form (including remove_suffix) requires no allocation.

Bullet 1 – I think – is out of scope for LSP, because it only applies to passing by value. Bullet 2 is satisfied by const_string.

Also, in the context of OO and its runtime polymorphism, when we talk about substitutability, we mean the behavior of virtual member functions, but not about constructors. Constructors are like factories: they are out of scope of LSV: it is obvious that you need different construction strategy for each subtype in the OO hierarchy, and whoever creates them needs to know implementation details of each such subtype. The use case in question:

boost::core::string_view(text).data() == text.data()

involves the object construction, so – under my interpretation – does not fall under the LSP reasoning.

akrzemi1 on Jul 8, 2022