Re: [boost] [system] Would it be possible to trial a breaking change to Boost.System and see what happens?

Subject: Re: [boost] [system] Would it be possible to trial a breaking change to Boost.System and see what happens?
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2018-01-12 18:19:54

On 01/12/18 20:43, Niall Douglas via Boost wrote:
> SG14 the low latency study has been looking into how to improve some
> design decisions in <system_error>. The relevant WG21 paper is
> https://wg21.link/P0824 and recent discussion can be found at
> https://groups.google.com/a/isocpp.org/forum/#!topic/sg14/j7tQybEjP5s.
> 
> What we'd like to do is to test whether some very mildly breaking
> changes to <system_error> are so mild as to affect no users, and on that
> basis propose WG21 to make those mildly breaking changes to
> <system_error> in the next C++ standard. And to test this, we'd like to
> modify Boost.System with those mildly breaking changes, ship a release
> and see how many users complain. If it is zero, we have evidence for
> WG21 that this is not a consequential change.
> 
> What we'd like to change is this:
> 
> 1. Stop treating code zero as always meaning success irrespective of
> category. This remedies a defect where some custom error code domains
> cannot be represented by error_code due to using value 0 for an error.
> In actual code, I've never seen anyone ever use comparison to anything
> but a default constructed error code, so I think this will be safe.

In my code I'm using contextual conversion to bool extensively, e.g. "if 
(err)" or "if (!err)". This is currently equivalent to comparing against 
0, so I assume it will be broken by this change. So, you can count me 
right now as the one (loudly) complaining, among many others, I think.

> 2. Success becomes solely the default constructed error code, which is
> code zero and a new category of "null_category". This is internally
> represented by { 0, nullptr }, and thus makes the default constructor
> trivial which is highly desirable as it eliminates any compiler magic
> static fencing for the default constructor. Right now the default
> constructor uses system_category with value 0, and I suspect no code
> will notice this change either.

This breaks "category()" as it returns a reference now. Also, 
"message()" has to check for a null category now.

> 3, Make the default constructor constexpr, now possible. No code should
> notice a difference, except less runtime code will be generated.

Not sure there will be any difference in the generated code, since the 
constructor still has to initialize the int and the pointer. Optimizers 
are already able to propagate constants even if the code is not constexpr.

The real benefit, it seems, is the ability to use error_code in constant 
expressions. I'm not sure how useful this is, I've never had a case 
where I needed this.

> 4. error_code::message() returns a std::string_view instead of
> std::string if running C++ 17 or better. This lets us remove the
> <string> dependency, and thus stop dragging in half the STL with
> <system_error> which in turn makes <system_error> actually useful to the
> embedded systems crowd. The guess here is that std::string_view has such
> excellent interoperation with std::string that I think 99% of code will
> compile and work perfectly without anybody noticing a thing.

This will only work if "error_category::message()" returns a string from 
a static storage. It will not allow relying on strerror_r or similar 
API, for example. This will make migration problematic if users define 
their own error categories that rely on external API like that.

> I appreciate that this would undo Peter Dimov's excellent work at having
> Boost.System alias <system_error> under C++ 11 and later. I also
> appreciate that we are deliberately going to potentially break end user
> code. But my personal guess is that in practice, the breakage will not
> be noticed by 99% of code out there right now. It'll just compile
> against the improved Boost.System and everything will work as before.
> 
> Boost was originally intended as an incubator for C++ standard changes.
> This ticks that box perfectly.
> 
> Thoughts on feasibility?

Even besides that my code is likely to be broken by this, I think this 
is a really bad idea. Sure, Boost was concieved as the playground for 
the features that are potentially later included into the standard, but 
it is also a major C++ library that is widely used in the industry. 
Breaking users' code willy-nilly does not do any good neither to Boost 
nor to its users. So no, no breaking changes to see how bad its smells 
afterwards, please.