Rather than change the meaning of the (ambiguous but no doubt ubiquitous) bool conversion, might it not be better to offer a new method? if (ec.is_error()) { // ... } is_error() could consult the error_category. or better, ADL: if (is_error(ec)) { // ... } re performance arguments: The only time the performance cost a virtual call is relevant is when the function is called in a tight loop. error_code is the result of a system or system-like call. In a reasonable program, it's never called in a tight loop. On 16 January 2018 at 14:30, Andrey Semashev via Boost < boost@lists.boost.org> wrote:

On 01/16/18 02:37, Robert Ramey via Boost wrote:

...

On 1/15/18 1:18 PM, Niall Douglas via Boost wrote:

That leaves the request to fix "if(ec) ..." which right now returns true

...

if the value is 0, despite that much code writes "if(ec) ..." to mean "if error then ...". There is also the issue of error coding schemes not being able to have more than one success value, which usually must be 0. That's the remaining discussion point.

I apologize for parachuting into the discussion ... But ...

Can't one just have "if(ec) ..." invoke a syntax error?

Obviously, this will break lots of code for no good reason. I think, the understanding of the meaning of such tests is more or less established. The problem is to make it support additional use cases without penalizing the existing code too much.

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

On 01/16/18 17:06, Peter Dimov via Boost wrote:

Richard Hodges wrote:

...

Rather than change the meaning of the (ambiguous but no doubt ubiquitous) bool conversion, might it not be better to offer a new method?

if (ec.is_error()) ...

Or ec.failed().

I doubt that we'll be able to change the meaning of `if(ec)` in the standard. Although who knows, it's a compatible change in the case nobody overrides error_category::failed. Which they don't because it doesn't exist yet.

I don't much like `if(ec)`, it's a bit counter-intuitive that it means "failed" and not "succeeded". But it's short and idiomatic.

If you spell it "if (err)" or "if (error)", it is very self-explanatory, IMHO. "if (error.failed())" makes it more verbose without adding any clarity.

On 1/16/18 5:30 AM, Andrey Semashev via Boost wrote:

On 01/16/18 02:37, Robert Ramey via Boost wrote:

...

On Can't one just have "if(ec) ..." invoke a syntax error?

Obviously, this will break lots of code for no good reason.

Truth is - If I see this I don't know what it actually means. I bet that half the code that would fail to compile is wrong. So I would argue that this would UNbreak lots of code.

I think, the understanding of the meaning of such tests is more or less established.

not to me. But maybe that's just me. Robert Ramey

The semantics of this `if(ec)` is "conditionally well-defined": provided that you make sure that in all error categories you use throughout your program zero indicates success and non-zero values indicate failure, then (and only then) does this construct test if `ec` represents a failure.

Forgive me for the noise in case I've missed something, but if the primary concern is that we want to be able to support non-zero success codes, why not just make the error_category carry the int representation of success instead of assuming that it's always zero? It's not as generic as introducing a new virtual that allows one to do whatever (for example there would still be one blessed value, so you couldn't have different 'success' values), but it's almost certainly faster than a virtual call, and might satisfy most of the use cases. -- chris

2018-01-16 15:59 GMT+01:00 Niall Douglas via Boost :

...

The semantics of this `if(ec)` is "conditionally well-defined": provided that you make sure that in all error categories you use throughout your program zero indicates success and non-zero values indicate failure, then (and only then) does this construct test if `ec` represents a failure.

Some programs can guarantee that, e.g. if they only use the default category on Unix.

However there are plenty of C error coding schemes with many values for success e.g.

SUCCESS_QUOTA_LOW SUCCESS_SLOWPATH SUCCESS_VIA_BROKER

... and so on. C++ 11's error_code's `if(ec) ...` doesn't handle these, and as Chris mentioned, it was never designed to do so. Though error_code itself handles them perfectly fine.

Originally, `if(ec)` meant "if error code". Of course, recent C++ isn't so sloppy anymore, we no longer should write `if(x)` when we mean `if(x != 0)`. And clang-tidy et al warns now appropriately.

That cultural shift in best practice is part of the cause of this debate with `if(ec) ...`. The programmer is writing that, and usually expecting it means something that it currently does not. After all, testing a smart pointer by boolean check means "is the smart pointer pointing to something?"

So are we reaching a consensus towards or away from the proposed change?

How about annotating the conditional conversion to bool deprecated? It still compiles and works as before, but with compiler flag `-Werror=deprecated` can be turned into compiler error on demand, and then you are forced to rewrite to either: ``` if (ec.value() != 0) ``` or: ``` if (ec.failure()) ``` ? Regards, &rzej;

2018-01-16 16:39 GMT+01:00 Andrey Semashev via Boost :

On 01/16/18 18:35, Andrzej Krzemienski via Boost wrote:

...

How about annotating the conditional conversion to bool deprecated? It still compiles and works as before, but with compiler flag `-Werror=deprecated` can be turned into compiler error on demand, and then you are forced to rewrite to either:

``` if (ec.value() != 0) ```

This is not correct if we're going to accept Niall's proposal.

Correct. This is an alternative rather than complement to Niall's proposal.

or:

...

``` if (ec.failure()) ```

?

I really don't see the point of this.

This attempts to make everyone happy. You can still use the old syntax (conversion to bool) with old semantics. Others can request the compiler to look for potentially buggy usages of conversion operator. There is a way to check fast for zero-code (regardless if it means failure of success -- and you state your intentions explicitly) and we get function `failure()` that explicitly says you want to check for failure condition even at the expense of a virtual function call. Regards, &rzej;

On 1/16/18 8:56 AM, Nevin Liber via Boost wrote:

On Tue, Jan 16, 2018 at 9:47 AM, Andrzej Krzemienski via Boost < boost@lists.boost.org> wrote:

...

This attempts to make everyone happy.

Then the attempt to *annotate the conditional conversion to bool deprecated* fails.

While it makes the people who love verbosity happy, it does so at a cost of breaking perfectly correct, well-tested code, and I suspect a large amount of code at that.

a) True - it would break code by invoking a compile time error b) which would be trivial to fix c) and likely in some cases smoke out some errors Heck, it even breaks Boost code. I would guess that

filesystem, asio and dll are some of the biggest users of error_code, and not only do users of those libraries regularly use "if (ec)", those libraries themselves internally use the same construct.

d) So we would find some new bugs in Boost as well. Sounds like a feature to me. Robert Ramey

James E. King, III wrote:

If you decide to change system_error / error_code please make it header-only.

We went over this last time, and touched the question again this time; the specification of system_category() requires that a reference to the same object is returned, and in the presence of DLLs, the only way to guarantee this is to make Boost.System itself a DLL. If we adopt a more relaxed attitude and change operator== for error_category to match (by using a similar technique there to what MS does in their ), we might be able to get header-only working in practice in this scenario (if one doesn't pay attention to the actual address of the category.) But even if we do fix the header-only case, switching to header-only by default would still be a major change.

On 1/16/18 10:06 AM, Nevin Liber via Boost wrote:

For people who don't have to pay. Churn for the sake of churn is costly for those who have to pay for testing, external software verification, etc.

I don't think it's just for the sake of churn. I think it's to improve program correctness by replacing ambiguous and opaque implicit behavior with unambiguous explicit behavior which is obviously correct on it's face without having to go look it in some documentation or source code somewhere else.

...

c) and likely in some cases smoke out some errors

Please provide some evidence to back up this assertion.

LOL - I can't "prove" that its going to smoke out some error. But that's not the point. One can't "prove" that the current usage is correct by looking at it. My experience is that something which is not obviously correct eventually comes to be seen as incorrect.

If the premise is that filesystem, asio and dll and the dominant use cases,

This is not a premise for me.

I'm not seeing any errors.

Of course you're not. The current API hides any which might be there. That's exactly my complaint. You cannot prove there are no errors in the usage simply by observation.

And if that isn't the premise, then what are the dominant use cases for error_code?

I see error_code as a way to reconcile varying lists of error codes in a systematic way which will avoid confusion which often leads to subtle, hard to find, bugs. Certainly I see it as much more general then three boost libraries.

...

Heck, it even breaks Boost code. I would guess that

...

filesystem, asio and dll are some of the biggest users of error_code, and not only do users of those libraries regularly use "if (ec)", those libraries themselves internally use the same construct.

The fact that something is in wide usage is not enough to convince me that it's a good idea.

...

d) So we would find some new bugs in Boost as well. Sounds like a feature to me.

My spot-checking of this idiom in Boost finds no bugs. Please provide evidence of a bug in Boost caused by this.

see above

Heck, Boost example code advertises this is the correct way to use error_code.

Maybe so, but that doesn't convince me that it's a good idea. Robert Ramey

2018-01-16 19:58 GMT+01:00 Andrey Semashev via Boost :

On 01/16/18 21:22, Robert Ramey via Boost wrote:

...

I don't think it's just for the sake of churn. I think it's to improve program correctness by replacing ambiguous and opaque implicit behavior with unambiguous explicit behavior which is obviously correct on it's face without having to go look it in some documentation or source code somewhere else.

There's nothing ambuguous about the conversion operator, as it is specified in the standard, and I find the syntax quite intuitive.

I disagree here. The sole fact that we are pursuing this thread proves that contextual conversion to bool for error_code is ambiguous. You get professional programmers that were convinced or expect that this conversion tells failure from success. You have others that say they have designed error_code with the intention in mind that 0 has nothing to do with "success". The fact that its semantics are clearly described in the standard does not resolve ambiguity or error prone-ness.The semantics of the following code are also well defined: ``` int i = f(); // ... if (i = 0) // just one = i = g(); ``` Maybe I know what I am doing. But compilers nonetheless chose to issue a warning here, because it is _likely_ that the program does something else than intended. I am looking at the problem from the perspective of a programmer that works on the same code base with 20 other programmers. When I review the code I need to know what the intentions of my colleague was. I need it expressed so that I do not have concerns whether he is doing it deliberately or accidentally. You say you find the syntax quite intuitive. But do you also find the semantics intuitive? When you type or read `if(ec)` do you interpret it as "if operation failed" or as "if operation returned code of numeric value zero, regardless of the domain and regardless of whether in this domain zero means success or failure"?

You're not improving correctness by replacing it with a regular method. Instead, you're adding verbosity.

It is not about correctness, or rather not directly about correctness. It is about stating your intentions unambiguously in the code. So that it does not only compile to the correct code, but also clearly communicate your intentions.

Whether that is a good thing or not is a matter of taste. Personally, I choose the shorter syntax in this case.

True, in the end it is just a choice.

Also, simply replacing one construct with another doesn't "fix" the code.

True, but it is not about "fixing", it is about expressing your intent. Given that conversion to bool has semantics confusing to many programmers, and that there will be a reluctance to change it for the fear of breaking backwards compatibility or degraded performance, providing a different name seems like a reasonable option. On the opposite, it has the potential of breaking it. It is clear to me that class `error_code` means two quite incompatible things. One is "just status" -- success or failure, or something in between. The second one is "reason for failure" (failure implied). The former was intended in the initial design. But there are things that indicate the latter: name "error", or conversion to bool, which conventionally means "valid or not?", "interesting or not?", "populated or not?". Users do not get clear message whether they should treat it as "reason for failure" or "just status". Regards, &rzej;

-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Andrey Semashev via Boost Sent: 16 January 2018 18:59 To: boost@lists.boost.org Cc: Andrey Semashev Subject: Re: [boost] Reforming Boost.System and round 2

<snip>

"There's nothing ambuguous about the conversion operator, "

You seem to have serendipitously invented a new word ' ambuguous ' that aptly describes this whole sorry mess ;-) Coming from outside C, (especially VMS where success == 1, not 0) if(ec) was and is, for me, definitely ambuguous! and other C-savvy posters in this thread confirm. No solution is going to be painless. I'd avoid trying to force any retrofit (because of downstream revalidation costs), but enforce use of Outcome (and >= C++11) on new libraries. Paul --- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830

Myself and Peter feel this is worth fixing despite the increased runtime overhead. Others, including one of its original inventors, feel it is too much overhead.

I must ask once again how do we know it is "too much" overhead? Where is the real-world program which would be impossible or difficult to write otherwise?

For some reason I never get an actual answer, and instead it's always something along the lines of "I'm an expert, trust me, I know". Well, I don't. Sorry.

A little unfair. I've supplied numbers to you before, but as with all such numbers, they're totally arbitrary and mostly meaningless. All anyone can really say is that with some real world application X, that Y was the case with that application. And that's about it. This particular debate comes down to how expensive is a virtual function call, and how frequently is code going to be doing `if(ec) ...`. As is fairly obvious by now, I consider virtual function call overhead to be pretty much in the stochastic noise of processor timings except on in-order CPUs [1]. As in, can be generally ignored as unimportant. I also think that `if(ec) ...` is not going to be called frequently, no more than twice per failing operation, and usually once. So to me, my proposed fix solves a real problem of correctness in lots of code out there in the wild which is currently subtly broken for virtually no cost. Therefore, it's a slam dunk. It should be implemented and the C++ standard upgraded to match. Obviously others disagree with me.

This applies to C++ exception handling and error handling in general. Yes, I know, there is overhead -- but where is the proof that 1) it matters, and 2) it's not worth it?

The "it matters" part depends on your attitude to the semantics behind observation of error codes. What does the code being 0 actually mean? Also, as Peter has explained several times now, composition of error_code returning functions in generic code where the category is unknown is hard without a generic facility to determine if some error_code means failure or not. For that alone, this change should be implemented. It breaks no existing code whatsoever, but fixes the semantics into what the programmer probably intended. Niall [1]: Of course raw benchmarks show virtual function calls are slower to no function call due to inlining in some artificial benchmark. But in real world code, the cost of a virtual function relative *to doing any work at all* is insignificant. While your doing work code is stalled on memory or whatever, the CPU is off busy executing virtual function calls and so on. Once you average it out, virtual function calls approach free in most real world code use cases on modern out-of-order CPUs. -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

On 01/16/18 16:54, Peter Dimov via Boost wrote:

Andrey Semashev wrote:

...

In the generated code I noticed that the compiler generated a check of whether the virtual function `failure` is actually `experimental::error_category::failure`. If it is, the code uses an inlined version of this function (otherwise, the actual indirect call is performed). So if you comment `code_category_impl::failure` the test succeeds and the indirect call is avoided. Here are the results for this case:

Experimental test: 71711 usec, 1394486201.559036 tests per second std test: 48177 usec, 2075679266.039812 tests per second

This is still a 1.5x difference.

That's pretty good. In practice, if the function does something nontrivial, this amount of overhead will be entirely lost in the noise.

Probably, although I think this number is a bit optimistic because the branch predictor is trained by the loop and that the indirect call is disfavored (it's been moved out of the loop). My worry is that we may not even achieve this number if the compiler is not capable or for some reason not able to perform the optimization. The worst case overhead of 6x looks much more grim.

On Tue, Jan 16, 2018 at 7:50 AM, Niall Douglas via Boost < boost@lists.boost.org> wrote:

You might be interested to learn that in a real world code base I tested, padding error_code's size to 64 bytes produced no statistically observable slowdown on Intel Ivy Bridge. As soon as it tipped over 64 bytes though, it became very noticeable, about 5%. That was for error_code_extended in Outcome v1, I wanted to see how much payload I could cram in there.

So when you say error_code comes with baggage over C error codes, I'll claim that you could in fact add lots more baggage still and probably see little effect.

This is interesting: There's a long-recognized need for instance-specific error parameter-values (e.g., a "key-value-map" for parameters, such as to include the `file-name` that we we failed to open). This seems to be the "number-one" reason to create user-specific wrappers around `std::error_code`. So, adding an 'std::unique_ptr<>` member to `std::error_code` would allow that; and could also solve the memory-transport issue to remove `std::string` from the `std::error_code` interface. Recall that currently (on *all* platforms that I'm aware of), `sizeof(std::error_code) == sizeof(int)+sizeof(void*)`. ...so, this idea would add `sizeof(unique_ptr<>)` to `std::error_code`. --charley

On 01/16/18 17:50, Niall Douglas via Boost wrote:

...

Given the above, unless LTO is used, I think the compiler will most often not be able to optimize the virtual function call.

As you noticed later in your reply, if a virtual function is marked final, the compiler inserts a check to see if the vtable's implementation matches and if so uses an inlined edition.

Being final has no effect if the final function declaration is not immediately visible to the compiler. The compiler optimization that I described is performed regardless of that markup.

...

Do I think this overhead is significant enough? Difficult to tell. Certainly I'm not happy about it, but I could probably live with the 1.5x overhead. However, it still results in code bloat and there is no guarantee this optimization will be performed by the compiler (or that it will be effective if e.g. my code always overrides `error_category::failure`). Thing is, every bit of overhead makes me more and more likely consider dropping `error_code` in favor of direct use of error codes. `error_code` already carries additional baggage of the pointer to error category.

You might be interested to learn that in a real world code base I tested, padding error_code's size to 64 bytes produced no statistically observable slowdown on Intel Ivy Bridge. As soon as it tipped over 64 bytes though, it became very noticeable, about 5%. That was for error_code_extended in Outcome v1, I wanted to see how much payload I could cram in there.

So when you say error_code comes with baggage over C error codes, I'll claim that you could in fact add lots more baggage still and probably see little effect.

Sure, what you call "significant overhead" depends on what you actually do. That's why there is no single answer to the question you asked in the initial message. But note that `error_code` is supposed to be the replacement for all kinds of error codes in the wild. There are APIs that use error codes with very simple, even trivial functions like setters or getters. If we are to claim `error_code` is suitable for those cases then it really should be very *very* close in costs to a simple `int` to be viable. So if it, for instance, cannot fit in a register to return efficiently then that's a problem. If it requires an indirect call to check for success, that's a problem too.

2018-01-16 17:05 GMT+01:00 Andrey Semashev via Boost :

On 01/16/18 18:56, Olaf van der Spek via Boost wrote:

...

On Mon, Jan 15, 2018 at 10:18 PM, Niall Douglas via Boost wrote:

...

Question: is this still considered too much overhead? Note that there is no longer ANY proposed breaking change with existing code. This is now 100% enhancement only.

Can't the is_error bool be stored in the ec object itself, at construction time?

Actually, I like this idea. I've modified my benchmark accordingly and it shows nearly identical performance as the current `std::error_code`:

Experimental test: 253654 usec, 394237820.022550 tests per second Experimental2 test: 46353 usec, 2157357668.327832 tests per second std test: 45981 usec, 2174811335.116679 tests per second

This is now getting closer to Boost.outcome, isn't it? Regards, &rzej;

2018-01-16 17:15 GMT+01:00 Andrey Semashev via Boost :

On 01/16/18 19:10, Andrzej Krzemienski via Boost wrote:

...

2018-01-16 17:05 GMT+01:00 Andrey Semashev via Boost < boost@lists.boost.org> :

On 01/16/18 18:56, Olaf van der Spek via Boost wrote:

...

...

Can't the is_error bool be stored in the ec object itself, at construction time?

Actually, I like this idea. I've modified my benchmark accordingly and it shows nearly identical performance as the current `std::error_code`:

Experimental test: 253654 usec, 394237820.022550 tests per second Experimental2 test: 46353 usec, 2157357668.327832 tests per second std test: 45981 usec, 2174811335.116679 tests per second

This is now getting closer to Boost.outcome, isn't it?

With my limited knowledge of Boost.Outcome, I don't think so. An outcome is supposed to carry a value or error, and `error_code` carries only the error code.

Boost.Outcome provides a number of tools. One of types, `result<void>` (maybe error of value, but value is void), is exactly this: a `bool` flag and an `error_code`. Regards, &rzej;

Also, a small technical detail. I believe carefully adding the flag to `error_code` doesn't increase its size. On 64-bit systems it fits nicely in the padding area. On 32-bit systems it could be merged into the category pointer, if we really wanted. Not sure `result<void>` does that.

I've taken great care to ensure Outcomes fit into the ABI stack spill size (32 bytes) where possible. Layout is always: T value; unsigned flags; E error; So if T were an int and E an error_code, size on 64 bit would be 24 bytes (error_code is 16 bytes), but just 16 bytes on 32 bit. result<void> is also 24 bytes, rather than 20 bytes, due to the error_code containing a pointer forcing alignment to the next 8 byte multiple. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

With my limited knowledge of Boost.Outcome, I don't think so. An outcome is supposed to carry a value or error, and `error_code` carries only the error code.

Actually the code() part of error_code is implicitly an outcome. Some code values are not errors, others are. It's just that error_code, being modelled on things like errno and GetLastError(), is implicitly archaic. If you'll forgive the expression, I feel we could do better than merely try to polish a turd from the 1950s. On 16 January 2018 at 17:15, Andrey Semashev via Boost < boost@lists.boost.org> wrote:

On 01/16/18 19:10, Andrzej Krzemienski via Boost wrote:

...

2018-01-16 17:05 GMT+01:00 Andrey Semashev via Boost < boost@lists.boost.org> :

On 01/16/18 18:56, Olaf van der Spek via Boost wrote:

...

...

Can't the is_error bool be stored in the ec object itself, at construction time?

Actually, I like this idea. I've modified my benchmark accordingly and it shows nearly identical performance as the current `std::error_code`:

Experimental test: 253654 usec, 394237820.022550 tests per second Experimental2 test: 46353 usec, 2157357668.327832 tests per second std test: 45981 usec, 2174811335.116679 tests per second

This is now getting closer to Boost.outcome, isn't it?

With my limited knowledge of Boost.Outcome, I don't think so. An outcome is supposed to carry a value or error, and `error_code` carries only the error code.

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

On 01/16/18 19:40, Peter Dimov via Boost wrote:

Andrzej Krzemienski wrote:

...

2018-01-16 17:05 GMT+01:00 Andrey Semashev via Boost :

...

On 01/16/18 18:56, Olaf van der Spek via Boost wrote:

...

Can't the is_error bool be stored in the ec object itself, at >> construction time?

Actually, I like this idea. I've modified my benchmark accordingly and > it shows nearly identical performance as the current `std::error_code`:

Experimental test: 253654 usec, 394237820.022550 tests per second Experimental2 test: 46353 usec, 2157357668.327832 tests per second std test: 45981 usec, 2174811335.116679 tests per second

This is now getting closer to Boost.outcome, isn't it?

It's a conforming implementation of the same (implied so far) specification. You have

   bool error_code::failed() const noexcept;

   Returns: category().failed( value() );    Remarks: implementations are allowed to cache the result of the expression.

For the record, I was thinking of (and testing) a different implementation that does not involve the `failed` call. Instead, I changed the `error_code` constructors: class error_code { int _value; bool _is_failure; const experimental::error_category *_category; public: constexpr error_code(int v, const experimental::error_category &cat) : _value(v), _is_failure(v != 0), _category(&cat) {} constexpr error_code(int v, bool is_failure, const experimental::error_category &cat) : _value(v), _is_failure(is_failure), _category(&cat) {} // Fixed operator bool, now asks category explicit operator bool() const noexcept { return _is_failure; } }; Calling (virtual) `error_category::failure` early is a waste if the user doesn't want to call that function himself. For instance, he may want to test specific error codes instead. Caching the result of `error_category::failure` on demand seems useless because most likely the error code will be tested for success only once. IMHO, this flag approach only makes sense when the flag is cheap and initialized early. For example, in the corresponding `make_error_code` overload. inline error_code make_error_code(my_error err) { return error_code(err, err < min_success || err > max_success, my_category); }

It can still be worth it because it's more likely to occur in a context where the compiler can see what category is being used. When you have

error_code ec( r, system_category() );

and system_category() is constexpr, the compiler knows with certainty what the call category().failed( r ) will do, so it will replace it with r != 0.

See f.ex. https://godbolt.org/g/6wGjCu

On 01/16/18 20:09, Peter Dimov via Boost wrote:

Andrey Semashev wrote:

...

Calling (virtual) `error_category::failure` early is a waste if the user doesn't want to call that function himself. For instance, he may want to test specific error codes instead.

It can still be worth it because it's more likely to occur in a context where the compiler can see what category is being used. When you have

   error_code ec( r, system_category() );

and system_category() is constexpr, the compiler knows with certainty what the call category().failed( r ) will do, so it will replace it with r != 0.

This will only be possible if the implementation of the category is visible at the point of `error_code` construction. I'm assuming that typically it is not. If the category is supposed to be visible then there is no reason to make that method virtual. Make it a regular function and change the constructor like this: template< typename Category, enable_if_t< is_base_of_t< error_category, Category > >

constexpr error_code(int v, const Category &cat) : _value(v), _is_failure(cat.is_failure(v)), _category(&cat) {} Although, honestly, I don't see much point in having that function in the category at this point. The code should be more clean if it tested the code against the enum values instead of `int`. So instead of this: bool my_category::is_failure(int err) { return static_cast< my_error >(err) < my_error::min_success || static_cast< my_error >(err) > my_error::max_success; } I would prefer this: error_code make_error_code(my_error err) { return error_code(static_cast< int >(err), err < my_error::min_success || err > my_error::max_success, my_category); } Note that this approach allows to generate `error_code` objects with the same category from multiple different enums. I think, someone mentioned a use case for this earlier in the discussion - when multiple enums basically map onto the common error domain, which has a single error category.

On 01/16/18 22:00, Niall Douglas via Boost wrote:

...

For the record, I was thinking of (and testing) a different implementation that does not involve the `failed` call. Instead, I changed the `error_code` constructors:

I feel no love for this design. What I want to do is construct an error code from some C error code returned to me without additional logic. I should not need to know whether the C error code is success or failure.

Someone has to decide what code is a success and what is not. In my code, it is the creator of `error_code`, who is supposed to be familiar with the domain. It should usually be `make_error_code`, a facility coupled with the error category and error domain.

It's true that if we use Outcome, the "failed" bit is already there so we don't need to ask the error_code at all. But the topic here, brought up by Niall, is error_code and not Outcome. Presumably, being the author of Outcome, he wouldn't ask for something he knows he doesn't need. :-)

Actually for Outcome, what I wanted from Boost.System was constexpr construction (got that) and removal of <string> (didn't get that). Outcome is useful precisely as a means to not have this problem with error_code. So the fix being discussed is not needed by Outcome. My interest in it is purely due to code written using Outcome which works with error codes. There is a second purpose though. Given my failure to remove <string>, I think it is now inevitable that I'll need to go implement a or something like that. It'll fix the header-only use problems and dragging in the STL allocator machinery. It'll also be more reliable for semantic comparisons. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

On 01/16/18 21:41, Niall Douglas via Boost wrote:

...

...

Can't the is_error bool be stored in the ec object itself, at construction time?

Actually, I like this idea. I've modified my benchmark accordingly and it shows nearly identical performance as the current `std::error_code`:

Experimental test: 253654 usec, 394237820.022550 tests per second Experimental2 test: 46353 usec, 2157357668.327832 tests per second std test: 45981 usec, 2174811335.116679 tests per second

Are you still calling the virtual function, but now in the constructor?

No, the flag is initialized in the `error_code` constructor. By default it is inferred from the error code value, but the user can specify the flag explicitly.

I didn't see you post any source code anywhere.

I posted an excerpt later in the discussion. For convenience, I'm attaching the full code here.