Opinions on introduction of string_view in Core
Hi, Recently, a new implementation of string_view was introduced in Boost.Core: https://github.com/boostorg/core/blob/d5bd40e5280487fb29a108eb42e6c4f0bef690... Most of you probably know there is also a string_view implementation in Boost.Utility: https://github.com/boostorg/utility/blob/05e0d1688dcfcd3fdd65bdb6884b7edd1ad... 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/4c54e4ca4c31f18ce2e451e8d0abf88054242d5... 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/95924b1329da49e70c9c8485b87e066895db...
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
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:
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.)
On Sat, Oct 16, 2021 at 12:02 PM Peter Dimov via Boost <boost@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
On Sat, 16 Oct 2021 at 21:01, Peter Dimov via Boost <boost@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:
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 hodges.r@gmail.com office: +44 2032 898 513 home: +376 861 195 mobile: +376 380 212
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.
On Oct 17, 2021, at 3:33 PM, Peter Dimov via Boost <boost@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
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.
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).
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.
On Mon, Oct 18, 2021 at 7:19 AM Marshall Clow via Boost <boost@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
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
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.
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.
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.
On Sun, Oct 17, 2021 at 5:33 PM Gavin Lambert via Boost <boost@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
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 <boost@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.
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.
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.
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.
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.
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.
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.
On Sun, Oct 17, 2021 at 6:21 PM Andrey Semashev via Boost <boost@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
On Sun, Oct 17, 2021 at 7:22 PM Vinnie Falco via Boost < boost@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)?
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.
On 10/18/21 5:21 AM, Vinnie Falco wrote:
On Sun, Oct 17, 2021 at 6:21 PM Andrey Semashev via Boost <boost@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.
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 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?
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.
On Oct 19, 2021, at 6:13 PM, Vinnie Falco via Boost <boost@lists.boost.org> wrote:
On Tue, Oct 19, 2021 at 6:08 PM Gavin Lambert via Boost <boost@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
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.
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; };
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.
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).
On Sun, Oct 17, 2021 at 5:47 PM Andrey Semashev via Boost <boost@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
On 10/18/21 3:57 AM, Vinnie Falco wrote:
On Sun, Oct 17, 2021 at 5:47 PM Andrey Semashev via Boost <boost@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.
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.
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 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.)
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.
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
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.
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
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.
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.
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.
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.
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
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.
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
On Tue, Oct 19, 2021 at 12:54 AM Julien Blanc via Boost <boost@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
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 (rainerd@eldwood.com)
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.
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.)
В письме от вторник, 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.
On Mon, Oct 18, 2021 at 6:13 PM Andrey Semashev via Boost <boost@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
participants (12)
-
Andrey Semashev -
Bjorn Reese -
Emil Dotchevski -
Gavin Lambert -
Glen Fernandes -
Julien Blanc -
Marshall Clow -
Peter Dimov -
Rainer Deyke -
Richard Hodges -
Vinnie Falco -
Дмитрий Архипов