[boost] Opinions on introduction of string_view in Core

[Andrey Semashev via Boost]

Hi,

Recently, a new implementation of string_view was introduced in Boost.Core:

https://github.com/boostorg/core/blob/d5bd40e5280487fb29a108eb42e6c4f0bef690d8/include/boost/core/string_view.hpp

Most of you probably know there is also a string_view implementation in
Boost.Utility:

https://github.com/boostorg/utility/blob/05e0d1688dcfcd3fdd65bdb6884b7edd1ad2c93c/include/boost/utility/string_view.hpp

The main motivation for introducing the new implementation is adding
conversions to/from std::string_view, which were proposed to
Boost.Utility and rejected a while ago:

https://github.com/boostorg/utility/pull/51

Another notable deviation of boost::core::string_view from
std::string_view and boost::string_view is that it only accepts the
character type in template parameters and not char_traits. This choice
was made to simplify implementation.

I have concerns that introducing yet another string_view type (3rd in
Boost, after boost::string_ref and boost::string_view, not counting
std::string_view) is not good for the ecosystem and interoperability, as
this will require adding explicit support in some code bases. One
example that I readilly have is Boost.Log:

https://github.com/boostorg/log/blob/4c54e4ca4c31f18ce2e451e8d0abf88054242d57/include/boost/log/utility/formatting_ostream.hpp#L558-L710

std::string_view was supposed to make user's code simpler by removing
the need to provide a multitude of function overloads accepting string
arguments. boost::string_view (and boost::string_ref before it) was a
prototype implementation before introduction of std::string_view and
served a similar purpose. In the age with std::string_view,
boost::string_view is still useful as a drop-in replacement for
std::string_view to lower C++ requirements in libraries and projects,
like this:

#if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)
using std::basic_string_view;
using std::string_view;

using std::string_view;


#else
using boost::basic_string_view;

using boost::string_view;


using boost::string_view;


#endif

Introducing boost::core::string_view, which is not a drop-in
replacement, does not make users' code simpler as it requires adding yet
another set of overloads and specializations to users' APIs.

I should stress that I'm not opposing because I don't want to make
changes to Boost.Log, I'm opposing because I don't think those changes
will be for the better, and because I view this as potentially
destructive to the wide adoption of one common string_view type (be it
std::string_view or boost::string_view, depending on the conventions
taken in the project) to accept string arguments.

Please, express your opinions on this matter, in particular:

- Do you think it is good/ok/bad to have yet another string_view in Boost?
- Is it ok that boost::core::string_view is not a drop-in replacement
for std::string_view? Should it be changed to be one?
- Should we, perhaps, do something with boost::string_view from
Boost.Utility? Deprecate?
- Should we reopen the discussion to add conversion to/from
std::string_view to boost::string_view, which led to this fork? Perhaps,
hold a vote to make this change as widely requested?
- Any other course of action or thoughts?

Thanks.

PS: This post was made after an initial discussion in this commit:

https://github.com/boostorg/core/commit/95924b1329da49e70c9c8485b87e066895db2d64

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Glen Fernandes via Boost]

On Sat, Oct 16, 2021 at 2:17 PM Andrey Semashev wrote:
> Please, express your opinions on this matter, in particular:
>
> - Do you think it is good/ok/bad to have yet another string_view in Boost?
> - Is it ok that boost::core::string_view is not a drop-in replacement
> for std::string_view? Should it be changed to be one?
> - Should we, perhaps, do something with boost::string_view from
> Boost.Utility? Deprecate?
> - Should we reopen the discussion to add conversion to/from
> std::string_view to boost::string_view, which led to this fork? Perhaps,
> hold a vote to make this change as widely requested?
> - Any other course of action or thoughts?

I would prefer that the boost::string_view in Boost.Utility have these
conversions.

A vote is fine, but - that said - I want to be clear that this is
still up to Marshall, i.e. I don't see this as a matter of a vote
overruling him, but rather it might help in convincing him to change
his mind.

Note that boost::string_view has the more optimal output operator
implementation (via Boost.IO which Boost.Core cannot depend on).

I would prefer one Boost string_view be the best, rather than have two
that aren't.

Glen

[Peter Dimov via Boost]

Andrey Semashev wrote:
> The main motivation for introducing the new implementation is adding
> conversions to/from std::string_view, which were proposed to Boost.Utility
> and rejected a while ago:
>
> https://github.com/boostorg/utility/pull/51

The scenario being addressed here is that of a Boost library that does not
require C++17, yet its users want to use std::string_view when interacting
with the library. That is, the library API is

string_view api_function( string_view str );

and some of the end users aren't on C++17 and will use the library-provided
string_view, and others are on C++17 and want to use std::string_view, both
to pass it as an argument to `api_function`, and to store its result.

Currently, if the library uses boost::string_view, it can't receive
std::string_view arguments. This is fixable by adding an additional overload
that takes a `std::string_view`, but other use cases now become ambiguous,
and need to be addressed separately either by yet more additional overloads,
or by templating some of the overloads, which is fragile.

And, this doesn't solve the problem of the return value. No matter what is
returned, some of the users will not be able to store the return value in their
preferred string_view.

A string_view type that converts from and to std::string_view solves this
problem, as the API above now works for both passing, and storing the result
into, `std::string_view`. boost::core::string_view is this type.

The alternative is for every library that has the above problem to supply its
own string_view, which will be a copy of boost::core::string_view. This is
exactly why Core was created - to factor out duplicate functionality.

(Before you suggest that the library should use boost::string_view under
C++14 and std::string_view under C++17, note that this makes it impossible
to build the library under C++14 and use it from C++17, which is a common
scenario when using the system-provided Boost package.)

[Vinnie Falco via Boost]

On Sat, Oct 16, 2021 at 12:02 PM Peter Dimov via Boost
<bo...@lists.boost.org> wrote:
> string_view api_function( string_view str );

1. The function name does not make it clear that the parameter is a string view
2. The function returns a string view which is dangerous

Therefore, the entire board must resign.

Thanks

[Gavin Lambert via Boost]

On 17/10/2021 07:17, Andrey Semashev wrote:
> - Do you think it is good/ok/bad to have yet another string_view in Boost?

It is bad to add a duplicate type. It is worse that it has different
behaviour from the original and from std::string_view.

> - Should we, perhaps, do something with boost::string_view from
> Boost.Utility? Deprecate?
> - Should we reopen the discussion to add conversion to/from
> std::string_view to boost::string_view, which led to this fork? Perhaps,
> hold a vote to make this change as widely requested?

One or more of these things, probably.


I would have less of a problem if the addition had been in another
library (such as Boost.StringView2) with clear deprecation of the one in
Boost.Utility and intent to remove in the future, similar to
Coroutine/Coroutine2 and other existing examples.

Adding a "replacement" that is not a replacement to Boost.Core just
seems like the wrong thing to do, on multiple levels.

[Richard Hodges via Boost]

On Sat, 16 Oct 2021 at 21:01, Peter Dimov via Boost <bo...@lists.boost.org>
wrote:

> Andrey Semashev wrote:
> > The main motivation for introducing the new implementation is adding
> > conversions to/from std::string_view, which were proposed to
> Boost.Utility
> > and rejected a while ago:
> >
> > https://github.com/boostorg/utility/pull/51
>
> The scenario being addressed here is that of a Boost library that does not
> require C++17, yet its users want to use std::string_view when interacting
> with the library. That is, the library API is
>
> string_view api_function( string_view str );
>
> and some of the end users aren't on C++17 and will use the library-provided
> string_view, and others are on C++17 and want to use std::string_view, both
> to pass it as an argument to `api_function`, and to store its result.
>
> Currently, if the library uses boost::string_view, it can't receive
> std::string_view arguments. This is fixable by adding an additional
> overload
> that takes a `std::string_view`, but other use cases now become ambiguous,
> and need to be addressed separately either by yet more additional
> overloads,
> or by templating some of the overloads, which is fragile.
>
> And, this doesn't solve the problem of the return value. No matter what is
> returned, some of the users will not be able to store the return value in
> their
> preferred string_view.

Is this not fixable by adding a std::string_view conversion operator to
boost::string_view?

>
>
> A string_view type that converts from and to std::string_view solves this
> problem, as the API above now works for both passing, and storing the
> result
> into, `std::string_view`. boost::core::string_view is this type.
>
> The alternative is for every library that has the above problem to supply
> its
> own string_view, which will be a copy of boost::core::string_view. This is
> exactly why Core was created - to factor out duplicate functionality.
>
> (Before you suggest that the library should use boost::string_view under
> C++14 and std::string_view under C++17, note that this makes it impossible
> to build the library under C++14 and use it from C++17, which is a common
> scenario when using the system-provided Boost package.)
>
>
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

--
Richard Hodges
hodg...@gmail.com
office: +44 2032 898 513
home: +376 861 195
mobile: +376 380 212

[Peter Dimov via Boost]

Gavin Lambert wrote:
> I would have less of a problem if the addition had been in another library (such
> as Boost.StringView2) with clear deprecation of the one in Boost.Utility and

> intent to remove in the future, ...

StringView2 works for me, but what exactly is the difference being sought? The
only thing I see is that it will go through a review which will likely reject it.

> Adding a "replacement" that is not a replacement to Boost.Core just seems like
> the wrong thing to do, on multiple levels.

The alternative is for all libraries that need this string_view (such as JSON, the
upcoming URL, Beast maybe) to contain their own private copy. Which is what
Core has been designed to solve. Of course, the process is supposed to be for
the private duplicate copies to appear first, then be moved into Core, so this is
admittedly a bit of a shortcut.

[Peter Dimov via Boost]

Richard Hodges wrote:
> Is this not fixable by adding a std::string_view conversion operator to
> boost::string_view?

Yes it is. Marshall is opposed to adding implicit conversions - from, or to,
std::string_view - to boost::string_view.

I understand this stance of his because implicit conversions are kind of a
pain and a maintenance burden because new ambiguities will very likely
keep popping up and need to be squashed. But what needs to be done,
needs to be done.

[Glen Fernandes via Boost]

On Sun, Oct 17, 2021 at 6:27 PM Richard Hodges wrote:
>
> Is this not fixable by adding a std::string_view conversion operator to
> boost::string_view?
>

Everyone (that is part of this thread) wanted this. Marshall maintains
boost::string_view and he needs to be convinced that this is the best
thing for users.

Glen

[Gavin Lambert via Boost]

On 18/10/2021 11:28, Peter Dimov wrote:
> Gavin Lambert wrote:
>> I would have less of a problem if the addition had been in another library (such
>> as Boost.StringView2) with clear deprecation of the one in Boost.Utility and
>> intent to remove in the future, ...
>
> StringView2 works for me, but what exactly is the difference being sought? The
> only thing I see is that it will go through a review which will likely reject it.

The difference is the deprecation.

And if you think that a review would reject it, this suggests that it is
not a good change. (Duplicating existing functionality is not by itself
grounds for rejection, although it does raise the hurdles.)

It might be easier to pass a review if it did more fully implement
std::string_view's behaviour, where possible pre-C++17 (or documented
justifications for not doing so).

>> Adding a "replacement" that is not a replacement to Boost.Core just seems like
>> the wrong thing to do, on multiple levels.
>
> The alternative is for all libraries that need this string_view (such as JSON, the
> upcoming URL, Beast maybe) to contain their own private copy. Which is what
> Core has been designed to solve. Of course, the process is supposed to be for
> the private duplicate copies to appear first, then be moved into Core, so this is
> admittedly a bit of a shortcut.

That would be fine if boost::string_view did not already exist. But it
does.

The best choice seems to be to add the necessary conversions to the
existing boost::string_view, rather than adding a new one. It just
seems like you're trying to do an end-run around this because you're
having trouble convincing the maintainer that it's a good idea. But
unilaterally adding a conflicting type to an existing library without
review and without deprecation of the old type is a worse idea.

Otherwise, introducing as a separate library and letting the community
decide what is better (with review and then with usage) seems like the
way to go. Preemptively putting it in Boost.Core feels like bypassing
community choice, which goes against the spirit of Boost.

[Vinnie Falco via Boost]

On Sun, Oct 17, 2021 at 5:33 PM Gavin Lambert via Boost
<bo...@lists.boost.org> wrote:
> Otherwise, introducing as a separate library and letting the community
> decide what is better (with review and then with usage) seems like the
> way to go. Preemptively putting it in Boost.Core feels like bypassing
> community choice, which goes against the spirit of Boost.

Marshall doesn't want to change it, and if it fails review then users
of my libraries will continue to encounter headwinds when using
C++17's std::string_view. I really don't understand what is so
difficult about this. An increasingly common complaint of Boost is
that it doesn't play well with the std types that Boost so generously
helped design.

Please explain to me what users of Beast, Boost.JSON, eventually
Boost.URL should do if they want to use C++17 string_view, Marshall
decides against implementing the change, and there is no consensus to
accept Boost.StringView2?

Thanks

[Peter Dimov via Boost]

Gavin Lambert wrote:
> That would be fine if boost::string_view did not already exist. But it does.
>
> The best choice seems to be to add the necessary conversions to the existing
> boost::string_view, rather than adding a new one. It just seems like you're
> trying to do an end-run around this because you're having trouble convincing
> the maintainer that it's a good idea. But unilaterally adding a conflicting type
> to an existing library without review and without deprecation of the old type is
> a worse idea.
>
> Otherwise, introducing as a separate library and letting the community decide
> what is better (with review and then with usage) seems like the way to go.
> Preemptively putting it in Boost.Core feels like bypassing community choice,
> which goes against the spirit of Boost.

OK then.

I'll be removing core::string_view in a few days unless I hear opinions to the
contrary.

[Andrey Semashev via Boost]

On 10/18/21 3:43 AM, Vinnie Falco via Boost wrote:
> On Sun, Oct 17, 2021 at 5:33 PM Gavin Lambert via Boost
> <bo...@lists.boost.org> wrote:
>> Otherwise, introducing as a separate library and letting the community
>> decide what is better (with review and then with usage) seems like the
>> way to go. Preemptively putting it in Boost.Core feels like bypassing
>> community choice, which goes against the spirit of Boost.
>
> Marshall doesn't want to change it, and if it fails review then users
> of my libraries will continue to encounter headwinds when using
> C++17's std::string_view. I really don't understand what is so
> difficult about this. An increasingly common complaint of Boost is
> that it doesn't play well with the std types that Boost so generously
> helped design.
>
> Please explain to me what users of Beast, Boost.JSON, eventually
> Boost.URL should do if they want to use C++17 string_view, Marshall
> decides against implementing the change, and there is no consensus to
> accept Boost.StringView2?

Making your libraries explicitly support std::string_view, in addition
to boost::string_view, is one option. I realize this is more work for
you as the maintainer, but this is a valid solution nonetheless.

[Peter Dimov via Boost]

Andrey Semashev wrote:
> Making your libraries explicitly support std::string_view, in addition to
> boost::string_view, is one option. I realize this is more work for you as the
> maintainer, but this is a valid solution nonetheless.

Remember that libraries also need to return string_views.

[Vinnie Falco via Boost]

On Sun, Oct 17, 2021 at 5:47 PM Andrey Semashev via Boost
<bo...@lists.boost.org> wrote:
> Making your libraries explicitly support std::string_view, in addition
> to boost::string_view, is one option.

Do you perhaps have some example code to share? Peter gave a function signature:

> The scenario being addressed here is that of a Boost library that does not
> require C++17, yet its users want to use std::string_view when interacting
> with the library. That is, the library API is
>

> boost::string_view api_function( boost::string_view str );

What would this look like with "explicitly supporting std::string_view
in addition to boost::string_view" ?

Thanks

[Andrey Semashev via Boost]

On 10/18/21 3:49 AM, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>> Making your libraries explicitly support std::string_view, in addition to
>> boost::string_view, is one option. I realize this is more work for you as the
>> maintainer, but this is a valid solution nonetheless.
>
> Remember that libraries also need to return string_views.

Yes. Depending on the case, this can be worked around too. For example,
have a convention that if you accept a string_view, you return the same
kind of string_view as well. If you don't accept a string_view, let the
user choose what he wants. Pick a default, if that's too verbose.

And in case you're worried about linking compatibility, that's not a
problem if you maintain your ABI properly. Have all this configurable
stuff in the headers and keep your core neutral to the C++ version and
config choices. Gladly, the conversion between string_views is cheap.

I'm not saying it's perfect, but it sounds doable.

[Peter Dimov via Boost]

Andrey Semashev wrote:
> > Remember that libraries also need to return string_views.
>
> Yes. Depending on the case, this can be worked around too. For example,
> have a convention that if you accept a string_view, you return the same kind
> of string_view as well.

And now all code that used to pass string literals or std::strings is broken
because of ambiguity. And if you add overloads to fix the ambiguity
(remember that the whole purpose of string_view is to let you get by without
adding overloads), you can't return both string views at once.

> If you don't accept a string_view, let the user choose
> what he wants. Pick a default, if that's too verbose.

How would that work, exactly? Take the example function signature and
show us how it's done.

[Andrey Semashev via Boost]

On 10/18/21 3:57 AM, Vinnie Falco wrote:
> On Sun, Oct 17, 2021 at 5:47 PM Andrey Semashev via Boost
> <bo...@lists.boost.org> wrote:
>> Making your libraries explicitly support std::string_view, in addition
>> to boost::string_view, is one option.
>
> Do you perhaps have some example code to share? Peter gave a function signature:
>
>> The scenario being addressed here is that of a Boost library that does not
>> require C++17, yet its users want to use std::string_view when interacting
>> with the library. That is, the library API is
>>
>> boost::string_view api_function( boost::string_view str );
>
> What would this look like with "explicitly supporting std::string_view
> in addition to boost::string_view" ?

Add an overload like this:

#if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)

inline std::string_view api_function( std::string_view str )
{
boost::string_view res = api_function(
boost::string_view(str.data(), str.size()));
return std::string_view(res.data(), res.size());
}

#endif

Of course, make helper functions to convert between the string_views.
And, as I said, have a convention in case if you don't take a
string_view as an argument.

[Peter Dimov via Boost]

Andrey Semashev wrote:
> Add an overload like this:
>
> #if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)
>
> inline std::string_view api_function( std::string_view str )
> {
> boost::string_view res = api_function(
> boost::string_view(str.data(), str.size()));
> return std::string_view(res.data(), res.size());
> }
>
> #endif

As I said, all code that used to call the function with a std::string or
with a string literal is now broken under C++17.

[Andrey Semashev via Boost]

On 10/18/21 4:10 AM, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>> Add an overload like this:
>>
>> #if !defined(BOOST_NO_CXX17_HDR_STRING_VIEW)
>>
>> inline std::string_view api_function( std::string_view str )
>> {
>> boost::string_view res = api_function(
>> boost::string_view(str.data(), str.size()));
>> return std::string_view(res.data(), res.size());
>> }
>>
>> #endif
>
> As I said, all code that used to call the function with a std::string or
> with a string literal is now broken under C++17.

Ok, let it be

template< typename T >
inline typename enable_if<
is_same< T, std::string_view >,
std::string_view
>::type api_function( T str )
{
...

[Andrey Semashev via Boost]

On 10/18/21 4:05 AM, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>>> Remember that libraries also need to return string_views.
>>
>> Yes. Depending on the case, this can be worked around too. For example,
>> have a convention that if you accept a string_view, you return the same kind
>> of string_view as well.
>
> And now all code that used to pass string literals or std::strings is broken
> because of ambiguity.

Then you'd use SFINAE or template matching to resolve the ambiguity.

> And if you add overloads to fix the ambiguity
> (remember that the whole purpose of string_view is to let you get by without
> adding overloads), you can't return both string views at once.

Sorry, I didn't understand the "both string views" part.

>> If you don't accept a string_view, let the user choose
>> what he wants. Pick a default, if that's too verbose.
>
> How would that work, exactly? Take the example function signature and
> show us how it's done.

boost::string_view get_string();

template< typename String >
String get_string()
{
boost::string_view res = get_string();
return String(res.data(), res.size());
}

Look, I'm talking in abstract code here. Perhaps, I'm missing some
crucial use case that you and Vinnie are aware of that absolutely never
can work like this. Perhaps you could present such use case and we can
think how it could be improved.

[Peter Dimov via Boost]

Andrey Semashev wrote:
> Ok, let it be
>
> template< typename T >
> inline typename enable_if<
> is_same< T, std::string_view >,
> std::string_view
> >::type api_function( T str )
> {
> ...
> }

And now C++17 users are sad because when they call
the function with a literal or a std::string it returns a
boost::string_view, which they can't assign to a std::
string_view.

You'd probably have them use a conversion function
on each call. That's workable but does not exactly deliver
the best user experience.

No, the correct answer is that you write your own private
string_view and use that.

(Or tell your users to switch to C++23 I suppose.)

[Peter Dimov via Boost]

Andrey Semashev wrote:
> > How would that work, exactly? Take the example function signature and
> > show us how it's done.
>
> boost::string_view get_string();
>
> template< typename String >
> String get_string()
> {
> boost::string_view res = get_string();
> return String(res.data(), res.size());
> }

The function is

boost::string_view api_function( boost::string_view str );

If we apply the technique above,

api_function( "something" );

will return boost::string_view, and C++17 users will have the option
of using

auto r = api_function<std::string_view>( "something" );

Again, this works, but isn't exactly optimal. C++17 users are penalized
for no good reason.

[Vinnie Falco via Boost]

On Sun, Oct 17, 2021 at 6:21 PM Andrey Semashev via Boost
<bo...@lists.boost.org> wrote:
> Perhaps you could present such use case and we can
> think how it could be improved.

The easiest and most obviously correct solution is to change the
existing boost::string_view or add a new boost::core::string_view to
be seamlessly compatible with std::string_view. Period. All of this
nonsense with writing templates, and SFINAE, and playing games with
"returning the same thing as what was passed in" sounds nice on the
Boost mailing list but it is not what users want.

C++17 users who choose C++11 Boost libraries are penalized in several
ways. Boost has its own pmr::memory_resource. Boost has its own
error_code. Boost has its own string_view. Even Boost flavored Asio
uses boost::error_code instead of std::error_code. This creates
friction for integrating Boost into existing C++17 code bases, a very
common user complaint. Requiring C++17 instead of C++11 for a Boost
library is obviously a non-starter since that cuts out an enormous
swathe of users.

If we want Boost to stay in the game, it has to adapt its offering to
be more friendly and seamless with std library vocabulary types. Peter
has already done much of this work, making boost::error_code
seamlessly interoperable with std::error_code (thanks for that!). Now
we need to do the same thing for string_view. Forcing N libraries to
change all their function signatures because we are allergic to having
a fork of Boost.Utilities' string_view is a losing strategy.

Thanks

[Emil Dotchevski via Boost]

On Sun, Oct 17, 2021 at 7:22 PM Vinnie Falco via Boost <
bo...@lists.boost.org> wrote:
> If we want Boost to stay in the game, it has to adapt its offering to
> be more friendly and seamless with std library vocabulary types. Peter
> has already done much of this work, making boost::error_code
> seamlessly interoperable with std::error_code (thanks for that!).

Should boost::shared_ptr be seamlessly interoperable with std::shared_ptr?
Why (or why not)?

[Gavin Lambert via Boost]

On 18/10/2021 16:31, Emil Dotchevski wrote:

> On Sun, Oct 17, 2021 at 7:22 PM Vinnie Falco wrote:
>> If we want Boost to stay in the game, it has to adapt its offering to
>> be more friendly and seamless with std library vocabulary types. Peter
>> has already done much of this work, making boost::error_code
>> seamlessly interoperable with std::error_code (thanks for that!).
>
> Should boost::shared_ptr be seamlessly interoperable with std::shared_ptr?
> Why (or why not)?

Ideally, yes. It's not super hard to use the deleter mechanism to make
them mostly interoperable (though not perfectly seamless, if you're
inspecting use_counts).

I'm happy to contribute such a wrapper to Peter if he's so inclined,
since I have one lying around (though wouldn't be surprised if he
already had his own).


Having said that, it's not without caveats (which I'm willing to live
with but others might be less so), and there is some functionality that
boost::shared_ptr has that std::shared_ptr lacks, which can be useful at
times (notably enable_shared_from_raw and local_shared_ptr).

So for those reasons it might be preferable to have it be an opt-in (or
even customizable) behaviour rather than by default, because they're
*not* drop-in replacements for each other, unlike things like
std::error_code.

[Andrey Semashev via Boost]

On 10/18/21 4:26 AM, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>>> How would that work, exactly? Take the example function signature and
>>> show us how it's done.
>>
>> boost::string_view get_string();
>>
>> template< typename String >
>> String get_string()
>> {
>> boost::string_view res = get_string();
>> return String(res.data(), res.size());
>> }
>
> The function is
>
> boost::string_view api_function( boost::string_view str );

The code I posted was an illustration of the case when you don't accept
a string in the function.

> If we apply the technique above,
>
> api_function( "something" );
>
> will return boost::string_view, and C++17 users will have the option
> of using
>
> auto r = api_function<std::string_view>( "something" );
>
> Again, this works, but isn't exactly optimal. C++17 users are penalized
> for no good reason.

If you want to make std::string_view the default, you can do exactly
that using the same approach. Automatically or by user defining a macro.

[Andrey Semashev via Boost]

I'm not arguing that having the conversions in boost::string_view is a
better solution, I'm in favor of that change. I'm just trying to work
out a solution given that we don't have the conversion. Forking
boost::string_view does not like a solution to me.

[Andrey Semashev via Boost]

On 10/18/21 4:22 AM, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>> Ok, let it be
>>
>> template< typename T >
>> inline typename enable_if<
>> is_same< T, std::string_view >,
>> std::string_view
>> >::type api_function( T str )
>> {
>> ...
>> }
>
> And now C++17 users are sad because when they call
> the function with a literal or a std::string it returns a
> boost::string_view, which they can't assign to a std::
> string_view.

Ok, then make std::string_view the default in you API. Write down the
requirements you have and write the code accordingly, it does not look
impossible to me.

> You'd probably have them use a conversion function
> on each call. That's workable but does not exactly deliver
> the best user experience.
>
> No, the correct answer is that you write your own private
> string_view and use that.

Writing more string_view types is not the answer for the reasons I wrote
earlier. At some point you will have a problem of interoperability
between those types, only more of them.

> (Or tell your users to switch to C++23 I suppose.)

I'm not sure what C++23 has to do with this.

[Glen Fernandes via Boost]

On Mon, Oct 18, 2021 at 7:47 AM Andrey Semashev wrote:
>
> On 10/18/21 4:22 AM, Peter Dimov via Boost wrote:
> Writing more string_view types is not the answer for the reasons I wrote
> earlier. At some point you will have a problem of interoperability
> between those types, only more of them.
>
> > (Or tell your users to switch to C++23 I suppose.)
>
> I'm not sure what C++23 has to do with this.

C++23 could get
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1989r2.pdf
which means that std::string_view can be implicitly constructed from
another range (e.g. boost::string_view) without boost::string_view
needing a conversion operator. Similarly if boost::string_view adds
that range constructor, it could be implicitly constructed from a
range like std::string_view.

Glen

[Peter Dimov via Boost]

Andrey Semashev wrote:

> I'm not arguing that having the conversions in boost::string_view is a better
> solution, I'm in favor of that change. I'm just trying to work out a solution
> given that we don't have the conversion. Forking boost::string_view does not
> like a solution to me.

Why not?

[Peter Dimov via Boost]

Glen Fernandes wrote:
> On Mon, Oct 18, 2021 at 7:47 AM Andrey Semashev wrote:
> >
> > On 10/18/21 4:22 AM, Peter Dimov via Boost wrote:
> > Writing more string_view types is not the answer for the reasons I
> > wrote earlier. At some point you will have a problem of
> > interoperability between those types, only more of them.
> >
> > > (Or tell your users to switch to C++23 I suppose.)
> >
> > I'm not sure what C++23 has to do with this.
>
> C++23 could get
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1989r2.pdf

cppreference shows it as C++23

https://en.cppreference.com/w/cpp/string/basic_string_view/basic_string_view

so maybe it already got voted in? I haven't kept track.

[Glen Fernandes via Boost]

On Mon, Oct 18, 2021 at 9:13 AM Peter Dimov wrote:
>
> Glen Fernandes wrote:
> > On Mon, Oct 18, 2021 at 7:47 AM Andrey Semashev wrote:
> > > On 10/18/21 4:22 AM, Peter Dimov via Boost wrote:
> > > > (Or tell your users to switch to C++23 I suppose.)
> > > I'm not sure what C++23 has to do with this.
> >
> > C++23 could get
> > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1989r2.pdf
>
> cppreference shows it as C++23
> https://en.cppreference.com/w/cpp/string/basic_string_view/basic_string_view
> so maybe it already got voted in? I haven't kept track.

Ah, yeah, looks like it was adopted in the June 2021 meeting.

Glen

[Marshall Clow via Boost]

On Oct 17, 2021, at 3:33 PM, Peter Dimov via Boost <bo...@lists.boost.org> wrote:
>
> Richard Hodges wrote:
>> Is this not fixable by adding a std::string_view conversion operator to
>> boost::string_view?
>

> Yes it is. Marshall is opposed to adding implicit conversions - from, or to,
> std::string_view - to boost::string_view.
>
> I understand this stance of his because implicit conversions are kind of a
> pain and a maintenance burden because new ambiguities will very likely
> keep popping up and need to be squashed. But what needs to be done,
> needs to be done.

I have two concerns here, one technical and one philosophical.

The first one is, as Peter has said, is implicit conversions. Specifically, introducing ambiguity.
We had to resolve ~10 ambiguous cases when we added string_view to the standard library.
Adding a 3rd type into the mix will almost certainly add new ones.

The second one is about the long term use of boost::string_view.
My opinion is that it is a transitional library, useful for people on older systems which do not have std::string_view.
All things being equal, we (boost) should be encouraging people to transition away from boost::string_view to std::string_view when they can.
[ Note that I don’t think that’s necessarily true for boost::shared_ptr, which has additional functionality above and beyond std::shared_ptr. ]

On the other hand, boost::string_view has additional functionality w.r.t std::string_view, because of some odd (to me) choices by the standard committee (removing comparisons, for example).

— Marshall

[Peter Dimov via Boost]

Marshall Clow wrote:
> The second one is about the long term use of boost::string_view.
> My opinion is that it is a transitional library, useful for people on older systems
> which do not have std::string_view.
> All things being equal, we (boost) should be encouraging people to transition
> away from boost::string_view to std::string_view when they can.

That's exactly the point though. People (library users) want to use
std::string_view, and libraries that don't want to require C++17 don't
have a good solution to offer them.

C++14 is still default on most distributions, so that's what Boost libraries
get compiled with.

Andrey's pontifications to the contrary notwithstanding, the best way to
address this use case is to make the library use a converting string_view in
its interface. This means that no additional overloads are required, and
the library can be compiled with C++14 and then be used with std::string_view.

Ideally, this "library API" type would be called boost::string_view. But if
this isn't possible, boost::core::string_view is second best. And if _that's_
impossible, third best is for this type to be called boost::library::string_view
(which was my original suggestion for Boost.JSON.)

Libraries already have this as a typedef, which can be toggled to refer to
boost::string_view or std::string_view, but this is not a satisfactory solution
for the reasons already outlined.

So yes, having two string views may be bad, but the alternative is having four
of five.

> On the other hand, boost::string_view has additional functionality w.r.t
> std::string_view, because of some odd (to me) choices by the standard
> committee (removing comparisons, for example).

Comparisons? I think std::string_view has comparisons?

There's also starts_with/ends_with, but that's in the C++20 std::string_view.
It also seems to have acquired `contains`, which I added.

[Peter Dimov via Boost]

Gavin Lambert wrote:
> The best choice seems to be to add the necessary conversions to the existing
> boost::string_view, rather than adding a new one. It just seems like you're
> trying to do an end-run around this because you're having trouble convincing
> the maintainer that it's a good idea.

Yes and no, but mostly no.

We're having trouble convincing the maintainer on two levels. The first one - I'll
use the terms Marshall himself used - is the philosophical one, the need for the
feature. We've apparently been unable to communicate this need and everyone
so far has been hard at work trying to explain to us that this need doesn't exist.

So one way to communicate this is to actually start using a converting string_view
in the libraries that need it, say Boost.JSON, and then point to it.

The second level is technical. Implicit conversions are a pain. They create
ambiguities in places where one doesn't expect. The way to convince Marshall
that the conversions are worth it and will create more value than they will cost
is...

... yes, to actually start using a converting string_view in one library, say
Boost.JSON, gather experience and then point to it as proof that the conversions
"work".

This could in principle have been accomplished by adding a private string_view
to JSON, rather than one to Core. But the maintainers of Boost.JSON were
unwilling to commit to that because it's a lot of work and the component is
foundational and usable in more than one library, so it needs to reside
in a common place (so that Beast or URL don't need to depend on JSON for
the string_view because it's nothing to do with JSON.)

So here we are.

(Incidentally, the new and improved string_view is also faster on a number of
benchmarks I did, and can be made faster still.)

> Otherwise, introducing as a separate library and letting the community decide
> what is better (with review and then with usage) seems like the way to go.

I'm not really sure that this is a path that leads us where we want to end up.

We want the boost string_view type to be called boost::string_view, and not
boost::strview2::string_view. Adding a new library doesn't seem to me to be
the best way to achieve that goal, although I suppose it's possible to end up
there taking this detour as well.

But more importantly, while this new library is getting through the review
cycle, JSON et al can't use it, and we need them to try to use it in order for
us to have confidence that we want this library at all.

> Preemptively putting it in Boost.Core feels like bypassing community choice,
> which goes against the spirit of Boost.

Correct but doesn't really help anyone.

At this point, what I think we can do is for me to move boost::core::string_view
to boost::core::detail::string_view and remove it from the public Core
documentation. It might seem a bit stupid to remove documentation that's
already written, but it's probably better for it to not be exposed to users yet
so that they don't start depending on it. Meanwhile, JSON and friends can try
using it and we'll see what comes out of that.

[Gavin Lambert via Boost]

On 19/10/2021 03:34, Peter Dimov wrote:
> So yes, having two string views may be bad, but the alternative is having four
> of five.

Those are not the only options.

Another possibility is to define a boost::any_string_view which is *not*
a clone of {std,boost}::string_view but just does conversion duck-typing
boilerplate (i.e. it defines no methods of its own other than
conversions, and the only useful thing you can do with one is to assign
it to a {std,boost,other}::string_view).

Ideally, this is what would appear in Boost.Core.

This is somewhat auto-hostile, but that's not unprecedented (Boost.Units
is also auto-hostile).

[Peter Dimov via Boost]

Gavin Lambert wrote:
> On 19/10/2021 03:34, Peter Dimov wrote:
> > So yes, having two string views may be bad, but the alternative is
> > having four of five.
>
> Those are not the only options.
>
> Another possibility is to define a boost::any_string_view which is *not* a
> clone of {std,boost}::string_view but just does conversion duck-typing
> boilerplate (i.e. it defines no methods of its own other than conversions, and
> the only useful thing you can do with one is to assign it to a
> {std,boost,other}::string_view).
>
> Ideally, this is what would appear in Boost.Core.

That's an option, yes. It's not clear why you think that's a better option, though.
You get all the costs of the implicitly convertible thing, and none of the benefits
of it being a string_view.

Maybe because you think that administratively, it would be easier to sneak into
Core without a review? But that's actually not true, because a component that
is a portable implementation of a C++17 thing and which therefore has its
interface already specified in the standard has a better claim to Core than a
novel component.

> This is somewhat auto-hostile, but that's not unprecedented (Boost.Units is
> also auto-hostile).

Auto-hostile, and doesn't allow things like f().starts_with("http:"), or f() == "foo".

That's a common theme with all alternative suggestions so far - they make
user experience worse for no good reason apart from red tape.

[Andrey Semashev via Boost]

On 10/19/21 1:55 AM, Peter Dimov via Boost wrote:
>
> At this point, what I think we can do is for me to move boost::core::string_view
> to boost::core::detail::string_view and remove it from the public Core
> documentation. It might seem a bit stupid to remove documentation that's
> already written, but it's probably better for it to not be exposed to users yet
> so that they don't start depending on it. Meanwhile, JSON and friends can try
> using it and we'll see what comes out of that.

Having a detail component part of the API is kind of odd. How would you
document the APIs that would use boost::core::detail::string_view, and
how would you advise users to deal with it, e.g. when they accept it as
a returned value? I mean, if you don't document it, users won't be able
to use it as in your `f().starts_with("http:")` example.

As an idea, if the goal is to test and refine conversions in the sole
use case of passing arguments and returning values, then maybe that is
how this component should be advertised. Let's call it a
boost::string_arg, a type that is supposed to be only used as function
arguments and return types and that doesn't do anything but the
conversion to/from strings, while delegating everything else to
boost::string_view. Advise users to never use boost::string_arg (and
Boost libraries - to only use it in function arguments), and for regular
string operations use boost/std::string_view. Note that this would be a
public component.

On a practical level, boost::string_arg would derive from
boost::string_view and only implement conversions. When/if we're happy
with it, we can simply merge it with boost::string_view and make
boost::string_arg a deprecated typedef/alias. And yes, boost::string_arg
should be compatible with boost/std::string_view in terms of template
parameters for that to be possible.

There is an obvious problem with auto, and we're still at an impasse if
we end up not convincing Marshall to merge the conversions into
boost::string_view in the end, but at least we will have something less
ambiguous to users than having a dozen string view types.

[Peter Dimov via Boost]

Andrey Semashev wrote:
> Having a detail component part of the API is kind of odd. How would you

> document the APIs that would use boost::core::detail::string_view, ...

One option is

class string_view: public core::detail::string_view
{
public:

using core::detail::string_view::string_view;
};

The other is to document the API in terms of the library typedef, and never
mention core or detail. (Which means no changes at all because the API is
already so documented.)

> As an idea, if the goal is to test and refine conversions in the sole use case of
> passing arguments and returning values, then maybe that is how this
> component should be advertised. Let's call it a boost::string_arg, a type that is
> supposed to be only used as function arguments and return types and that
> doesn't do anything but the conversion to/from strings, while delegating
> everything else to boost::string_view. Advise users to never use
> boost::string_arg (and Boost libraries - to only use it in function arguments),
> and for regular string operations use boost/std::string_view. Note that this
> would be a public component.
>
> On a practical level, boost::string_arg would derive from boost::string_view
> and only implement conversions.

This is not what we want. We want a converting string_view that is called
boost::string_view. The way to get it is to make a converting string_view so
that we demonstrate the viability of the idea, then make boost::string_view
be it.

Deriving the new string_view from the old one is not the best way to get
there, because it's not the same thing and its behavior in actual code will
be different.

Admittedly, the above "opaque typedef" is not the same thing either, but
it's closer in behavior to what core::string_view is now.

[Vinnie Falco via Boost]

On Mon, Oct 18, 2021 at 6:13 PM Andrey Semashev via Boost
<bo...@lists.boost.org> wrote:
> How would you
> document the APIs that would use boost::core::detail::string_view

Like this:

namespace boost {
namespace json {

/** The type of string_view used by the library

This type is used when a non-owning reference
to a character buffer is passed by parameter
or returned by value.

For full details see
<a href="https://en.cppreference.com/w/cpp/string/basic_string_view"
>std::string_view (cppreference.com)</a>
*/
#ifdef BOOST_JSON_DOCS
using string_view = __see_below__
#else
using string_view = boost::core::detail::string_view;
#endif

} // json
} // boost

[Julien Blanc via Boost]

Le mardi 19 octobre 2021 à 04:33 +0300, Peter Dimov via Boost a écrit :

> This is not what we want. We want a converting string_view that is
> called
> boost::string_view. The way to get it is to make a converting
> string_view so
> that we demonstrate the viability of the idea, then make
> boost::string_view
> be it.

From the discussion, what i understand is what you want ideally is an
std::string_view for c++11/14.

> (Before you suggest that the library should use boost::string_view
> under C++14 and std::string_view under C++17, note that this makes it
> impossible to build the library under C++14 and use it from C++17,
> which is a common scenario when using the system-provided Boost 
> package.)

I'm sorry but i'm gonna raise that suggestion. While I agree it does
not solve the general use case, it looks to me like an acceptable
solution for boost.json. IIRC boost.json is advertised as a nearly
header-only library, where you have to include an hpp file once in your
project. So ABI mismatch should not hold there (by the way, i just
checked that it is the way it is packaged on my debian stable, no
binary for json is included).

Maybe some #defines could be added to avoid some mismatches, or to
force an ABI independantly from language level.

Regards,

Julien

[Gavin Lambert via Boost]

On 19/10/2021 18:42, Julien Blanc wrote:
> I'm sorry but i'm gonna raise that suggestion. While I agree it does
> not solve the general use case, it looks to me like an acceptable
> solution for boost.json. IIRC boost.json is advertised as a nearly
> header-only library, where you have to include an hpp file once in your
> project. So ABI mismatch should not hold there (by the way, i just
> checked that it is the way it is packaged on my debian stable, no
> binary for json is included).

Header-only does not avoid ABI ODR problems. It just moves where they
occur (which in some ways makes them a lot worse than built libraries).

I'm with Peter on that one: just throwing in a preprocessor-conditioned
typedef is not a reasonable solution.


Preprocessor-conditioned overloads *do* work for arguments (though is an
additional implementation burden since you would have to provide
multiple implementations of the same thing), but doesn't help for return
types, since sadly the return type does not participate in name mangling
or overload resolution. While not impossible to have ABI-preserving
alternative methods varying by return type, having one well-defined
return type is really the best solution for that.

(FWIW, I agree that boost::string_view should be that type, but I'm not
fond of the contortions being proposed to avoid changing the current
one. And I especially don't like the idea of trying to alias
boost::core::string_view as boost::string_view while the one in
Boost.Utility still exists; while that won't in itself cause ODR
problems, it will cause aggravating compile errors if both end up
included, which is highly probable. The current Boost.Core
implementation doesn't appear to be doing that, but it sounded like it
was being suggested.)

It's not too bad if all methods that return string_view also take a
string_view as argument; it's most problematic for those that need to
return string_view with no arguments.

Also, before you suggest template duck-typing for arguments so that you
can avoid implementing every method twice: templates are problematic
when you want to allow implicit conversions (such as from std::string
and constant strings), so you're likely to need even more overloads that
way. And pre-Concept template metaprogramming to make those overloads
actually work is frequently inscrutable and fragile.

[Julien Blanc via Boost]

Le 2021-10-19 08:26, Gavin Lambert via Boost a écrit :
> On 19/10/2021 18:42, Julien Blanc wrote:
>> I'm sorry but i'm gonna raise that suggestion. While I agree it does
>> not solve the general use case, it looks to me like an acceptable
>> solution for boost.json. IIRC boost.json is advertised as a nearly
>> header-only library, where you have to include an hpp file once in
>> your
>> project. So ABI mismatch should not hold there (by the way, i just
>> checked that it is the way it is packaged on my debian stable, no
>> binary for json is included).
>
> Header-only does not avoid ABI ODR problems. It just moves where they
> occur (which in some ways makes them a lot worse than built
> libraries).
>
> I'm with Peter on that one: just throwing in a
> preprocessor-conditioned typedef is not a reasonable solution.

I'm certainly biased toward our own use cases. Maintaining a medium
sized (<1Mloc) that needs to compile with compilers as old as gcc4.8 as
well as new compilers such as gcc11, and several boost versions, we
already have our base library filled with things such as:

#if __cplusplus >= 201703L
using filesystem = std::filesystem;
#else
using filesystem = boost::variant;
#endif

And then our code works on the compile time selected type. They're
usually close enough so that it works (and if it doesn't, we write the
required helper functions). The fact that we're always compiling the
whole target (embedded world) certainly helps avoiding ABI issues.

I don't know how widespread doing this is. But i would be surprised if
it were really uncommon.

> Preprocessor-conditioned overloads *do* work for arguments (though is
> an additional implementation burden since you would have to provide
> multiple implementations of the same thing), but doesn't help for
> return types, since sadly the return type does not participate in name
> mangling or overload resolution. While not impossible to have
> ABI-preserving alternative methods varying by return type, having one
> well-defined return type is really the best solution for that.

I'm not sure i was being clear here. I'm not talking about overloads,
but about replacements. Something like

#if __cplusplus >= 201703L
using string_view = std::string_view
#else
using string_view = boost::string_view
#endif

The interfaces are close enough so that there shouldn't be a need to
implement things twice in boost::json, which would always use
boost::json::string_view. You either got the c++14 boost::string_view
api, or the c++17 std::string_view api, but never both.

As a user, i fully supports what Vinnie said: users want to use stl
types as much as possible, not their boost equivalents. Every time a
boost library forces its users to use boostified versions for little or
no reasons, it is somewhat hostile to users. So a long term goal should
be to remove these, not add more...

Regards,

Julien

[Дмитрий Архипов via Boost]

В письме от вторник, 19 октября 2021 г. 08:42:15 MSK пользователь Julien Blanc
via Boost написал:

> I'm sorry but i'm gonna raise that suggestion. While I agree it does
> not solve the general use case, it looks to me like an acceptable
> solution for boost.json. IIRC boost.json is advertised as a nearly
> header-only library, where you have to include an hpp file once in your
> project. So ABI mismatch should not hold there (by the way, i just
> checked that it is the way it is packaged on my debian stable, no
> binary for json is included).

Boost.JSON is not advertised as a nearly header-only library. It's advertised
as a library that supports header-only use. By default it is still compiled as
normal. I've checked Debian Sid package search and I can see that it uses
Boost 1.74. JSON only appeared in 1.75, so that's why there's no binary on
your machine.

Looking at where the winds are blowing in this discussion, we'll have to
indeed add a string_view implementation to Json. Problems with other
approaches were already presented by Peter, so I won't repeat them. So, there
will be (at least one) new string_view in Boost, just not in Core.

[Rainer Deyke via Boost]

On 19.10.21 08:26, Gavin Lambert via Boost wrote:
> Header-only does not avoid ABI ODR problems.  It just moves where they
> occur (which in some ways makes them a lot worse than built libraries).

ODR violations can be prevented with namespace aliases.

#if __cplusplus >= 201703L
#define BOOST_JSON_NAMESPACE json_with_std_string_view
namespace BOOST_JSON_NAMESPACE {

using string_view = std::string_view;
}
#else

#define BOOST_JSON_NAMESPACE json_with_boost_string_view
namespace BOOST_JSON_NAMESPACE {

using string_view = boost::string_view;
}
#endif

using json = BOOST_JSON_NAMESPACE;
namespace BOOST_JSON_NAMESPACE {
// Library contents go here.
}

That said, compiling parts of the same program with different C++
standards is asking for trouble. I would expect ODR violations from the
standard library in that case.

--
Rainer Deyke (rai...@eldwood.com)

[Peter Dimov via Boost]

Rainer Deyke wrote:
> On 19.10.21 08:26, Gavin Lambert via Boost wrote:
> > Header-only does not avoid ABI ODR problems. It just moves where they
> > occur (which in some ways makes them a lot worse than built libraries).
>
> ODR violations can be prevented with namespace aliases.
>
> #if __cplusplus >= 201703L
> #define BOOST_JSON_NAMESPACE json_with_std_string_view
> namespace BOOST_JSON_NAMESPACE {
> using string_view = std::string_view;
> }

...

JSON already does that for its "standalone" mode, to support linking
"Boost" and "standalone" JSON in the same application because of
user demand.

> That said, compiling parts of the same program with different C++
> standards is asking for trouble. I would expect ODR violations from the
> standard library in that case.

People do that every day, sometimes without even realizing it.

[Gavin Lambert via Boost]

On 20/10/2021 01:27, Peter Dimov wrote:
>> That said, compiling parts of the same program with different C++
>> standards is asking for trouble. I would expect ODR violations from the
>> standard library in that case.
>
> People do that every day, sometimes without even realizing it.

Any time you're using a system-provided C++ library, you're doing this.
And that includes system-provided Boost.

It's fairly commonplace to compile an app with a higher C++ standard
than was used for the system libraries, because those tend to be built
with conservative defaults.

The reverse is also possible, but less common, since if a library
requires a higher C++ standard to build itself then this usually means
consumers of the library need at least that or higher too. Still, this
is not guaranteed; a library might be using some features only in its
source files and not its public headers.

And I've seen quite a few libraries where only a subset of source files
are compiled with a higher standard level than the others.

(Things are a bit different here between *nix and Windows. Most *nix
distros designate a One True C++ Compiler™ for their current release,
including a default C++ standard level and library, and most things are
built to this, so it's easier to rely on prebuilt system libraries.
Whereas on Windows there is no standard C++ compiler or library, so it's
more commonplace to either "build the world" and distribute private
binaries or to rely on the C or COM ABIs instead of the C++ ABI, since
those are more stable.)

[Gavin Lambert via Boost]

On 18/10/2021 15:21, Vinnie Falco wrote:
> The easiest and most obviously correct solution is to change the
> existing boost::string_view or add a new boost::core::string_view to
> be seamlessly compatible with std::string_view. Period. All of this
> nonsense with writing templates, and SFINAE, and playing games with
> "returning the same thing as what was passed in" sounds nice on the
> Boost mailing list but it is not what users want.

To be clear: I do agree with the above.

(Although Andrey's original post suggests that the current
boost::core::string_view is *not* seamlessly compatible with
std::string_view. I have not verified those concerns, but there'd need
to be very good justification for it if so.)

What I'm not clear on is the intended future of Boost.Utility's string_view.


If the intent is as a "change of ownership" for maintenance purposes
from Boost.Utility to Boost.Core, then the one in Boost.Utility should
be immediately deprecated, and (if sufficiently compatible, immediately;
otherwise in N boost releases) replaced with an alias typedef, then
later removed entirely (at which point Core can add an alias from
boost::string_view, but not before).

If the intent is to introduce a new alternative implementation without
cooperation from Boost.Utility, then it ought to be introduced via the
proper formal review process, as if a new library (though it need not
actually be implemented as such), so that the community can determine
the merits. The result might end up the same as above, or the two might
co-exist for longer until the users end up deciding.

(To some extent this thread can be considered a kind of informal review,
but it's likely less people are paying attention to it than would be the
case for a formal review.)

Or maybe the goal is to update the one in Boost.Utility instead of
introducing a new one -- but that doesn't appear to be the case.


It's long been a hole in Boost processes that new libraries require a
formal peer review process but once over that hurdle, new functionality
(including highly unrelated functionality, especially in a kitchen-sink
library like Core) have no such requirement, instead relying solely on
maintainers voluntarily requesting reviews or on people noticing commits
and raising concerns (as in this case). There are reasons for this --
and I don't disagree with most of those -- but there is a line
*somewhere* and I feel like this may have crossed it. I might be alone
in this, but from the general tone of this thread, I don't think I am.

[Vinnie Falco via Boost]

On Tue, Oct 19, 2021 at 6:08 PM Gavin Lambert via Boost
<bo...@lists.boost.org> wrote:
> What I'm not clear on is the intended future of Boost.Utility's string_view.

Marshall, O' Marshall; Where art thou?

[Marshall Clow via Boost]

On Oct 19, 2021, at 6:13 PM, Vinnie Falco via Boost <bo...@lists.boost.org> wrote:
>
> On Tue, Oct 19, 2021 at 6:08 PM Gavin Lambert via Boost
> <bo...@lists.boost.org> wrote:
>> What I'm not clear on is the intended future of Boost.Utility's string_view.
>
> Marshall, O' Marshall; Where art thou?

I’m here; I’m reading.
I even responded to this thread yesterday; though I’m not sure anyone noticed.

— Marshall

[Vinnie Falco via Boost]

On Mon, Oct 18, 2021 at 7:19 AM Marshall Clow via Boost
<bo...@lists.boost.org> wrote:
> I have two concerns here, one technical and one philosophical.
>
> The first one is, as Peter has said, is implicit conversions. Specifically, introducing ambiguity.
> We had to resolve ~10 ambiguous cases when we added string_view to the standard library.
> Adding a 3rd type into the mix will almost certainly add new ones.

Yeah and that's why we need this new string_view to go into core,
protestations against the lack of formal review notwithstanding. So we
can sort out these conversions.

> The second one is about the long term use of boost::string_view.
> My opinion is that it is a transitional library, useful for people on older systems which do not have std::string_view.
> All things being equal, we (boost) should be encouraging people to transition away from boost::string_view to std::string_view when they can.

Equivalent to saying that "we (boost) should be encouraging people to
transition away from C++11 in favor of C++17." Until C++11 has less
than 5% of the market, I certainly can't be requiring C++17 for any of
my libraries. And as of the latest language survey, C++11 is still
firmly ensconced as a Very Popular Version of the language.

This leaves us with a big problem. We are encountering headwinds
evolving a component that is vital for existing C++11 libraries.

> [ Note that I don’t think that’s necessarily true for boost::shared_ptr

> which has additional functionality above and beyond std::shared_ptr. ]

The set of use-cases for string_view is considerably larger than the
set of use-cases for shared_ptr in argument lists. Furthermore
std::shared_ptr is available in C++11 (which realistically is the
minimum that new libraries should target).

Thanks

[Vinnie Falco via Boost]

On Tue, Oct 19, 2021 at 12:54 AM Julien Blanc via Boost
<bo...@lists.boost.org> wrote:
> #if __cplusplus >= 201703L
> using filesystem = std::filesystem;
> #else
> using filesystem = boost::variant;
> #endif

> ...

> #if __cplusplus >= 201703L
> using string_view = std::string_view
> #else
> using string_view = boost::string_view
> #endif

This effectively creates 4 different libraries:

{ std::filesystem, std::string_view },
{ boost::variant, std::string_view },
{ std::filesystem, boost::string_view }, and
{ boost::variant, boost::string_view }

I am transitioning away from header-only in favor of compiled
libraries (or including the single-header definitions file src.hpp),
to be responsive to user feedback that Boost libraries "take a long
time to compile and use too many templates" (which sometimes, they
do). Linking to compiled libraries increases the complexity and error
rate of builds so the last thing I want to do is create multiple ABI
forks of my libraries because of #ifdefs on types used in public
interfaces, so this approach is a non-starter.

Thanks

[Vinnie Falco via Boost]

On Tue, Oct 19, 2021 at 7:50 PM Vinnie Falco <vinnie...@gmail.com> wrote:
> This effectively creates 4 different libraries:

My bad... two libraries.

[Peter Dimov via Boost]

Gavin Lambert wrote:

> What I'm not clear on is the intended future of Boost.Utility's string_view.

The future of Utility's string_view will be what we decide it to be. As already
stated, in order to build a strong case for the boost::string_view type to be
convertible and interoperable, we need to actually demonstrate that this
interoperable type is necessary, desirable, and can actually be made to work.

While it's possible, if not easy, to achieve the first two via discussions on the
list, the third one requires actual practical use, because we need to root out
all the ambiguities and show that the potential problems they create can be
solved adequately.

Once we have consensus that we want the interoperable type to be called
boost::string_view, we can decide how to do it, whether by adding
conversions to Utility's string_view, or by promoting Core's one and removing
the one in Utility.

The alternative option of leaving boost::string_view as is doesn't particularly
appeal to me. I think that it should (at least broadly) track the C++23 changes
to std::string_view, and that includes a converting constructor. So if we assume
a C++23 std::string_view and a C++23-compatible boost::string_view, each
will convert to the other using this constructor. I see no reason to make
users of C++17's std::string_view wait for C++23 in order to be able to take
advantage of this interoperability between the two string views, which is why
I think we should add a conversion to std::string_view now, rather than
waiting for std::string_view to add a conversion from, in 2023.

But even if the ultimate fate of boost::string_view is to remain as is, this doesn't
change anything today. In all three cases it starts with adding the converting
string_view _somewhere_ and then using it in Boost library interfaces.

[Bjorn Reese via Boost]

There is another option that should be considered.

Given that string views are cheap to copy, we could create an
intermediary string view class that has all the implicit conversions
and nothing else. The sole purpose of this, say, string_mediator is to
convert between std::string_view and boost::string_view.

This string_mediator becomes the interoperability type, and no change
is needed to either std::string_view nor boost::string_view.
Furthermore, no implicit conversions are needed in boost::string_view.

If you want your API to support implicit conversion, then you should
use string_mediator in your API.

If you wish to do string operations on the string_mediator, then it
should be converted into either std::string_view or boost::string_view.

The following is just a simple sketch of how it could look. Obviously
this should be changed into basic_string_mediator, and default, copy,
and move constructor/assignment should be considered.

class string_mediator {
public:
constexpr string_mediator(std::string_view other) noexcept
: data(other.data()), size(other.size()) {}

constexpr string_mediator(boost::string_view other) noexcept
: data(other.data()), size(other.size()) {}

operator std::string_view() noexcept {
return { data, size };
}

operator boost::string_view() noexcept {
return { data, size };
}

private:
std::string_view::pointer data;
std::string_view::size_type size;
};

[Gavin Lambert via Boost]

On 21/10/2021 11:19, Bjorn Reese wrote:
> There is another option that should be considered.
>
> Given that string views are cheap to copy, we could create an
> intermediary string view class that has all the implicit conversions
> and nothing else. The sole purpose of this, say, string_mediator is to
> convert between std::string_view and boost::string_view.
>
> This string_mediator becomes the interoperability type, and no change
> is needed to either std::string_view nor boost::string_view.
> Furthermore, no implicit conversions are needed in boost::string_view.
>
> If you want your API to support implicit conversion, then you should
> use string_mediator in your API.

Yes, that's what I suggested elsewhere (as boost::any_string_view).

It does have some drawbacks (it can't be used directly as an rvalue, you
need to cast or assign it to a "real" string_view type first).

I don't think Peter likes that option since his goal seems to be to
replace string_view entirely. (Which I'm not inherently opposed to; I
just think it's not being done the right way.)

It's not as good as updating the existing boost::string_view but that
option also seems to be off the table for whatever reason.

[Gavin Lambert via Boost]

On 21/10/2021 11:19, Bjorn Reese wrote:

>         constexpr string_mediator(std::string_view other) noexcept
>             : data(other.data()), size(other.size()) {}
>
>         constexpr string_mediator(boost::string_view other) noexcept
>            : data(other.data()), size(other.size()) {}

Also, it's not quite this straightforward -- since both of these are
also implicitly constructable from char pointers and std::strings, you
have to add constructor overloads for these conversions too, otherwise
it's more annoying for the consumer.

(Even if the compiler permitted two consecutive implicit conversions,
which it doesn't, that would be ambiguous in this case -- which is *why*
it doesn't.)

It's relatively trivial to add these extra constructors as well,
although the end result is that you've almost entirely duplicated the
implementation of string_view anyway, just without the string-like
convenience methods (which may also be annoying for the consumer).

Having the wrapper inherit from one of the two (naturally, this has to
be boost::string_view) fixes both of these problems and reduces the
amount of code required (as I think Peter suggested himself at one
point). It also more clearly indicates which operations are "missing"
from boost::string_view. I don't recall why he didn't like this option
either, other than "I already wrote the full replacement" (implied, not
quoted).