gnzlbg / static_vector

A dynamically-resizable vector with fixed capacity and embedded storage
https://gnzlbg.github.io/static_vector
167 stars 21 forks source link

Error handling using expected<T, std::some_exception> #34

Closed gnzlbg closed 5 years ago

gnzlbg commented 6 years ago

Ben Craig mentioned the possibility of just using expected<T, std::some_exception> for error handling. As mentioned in the Future extensions section, such an avenue can be explored with try_xxx methods, but maybe we can just change the signature of some of the methods. In any case, I think this should be a follow-up paper improving on the fundamental fixed_capacity_vector semantics, instead of trying to propose this in the first fixed_capacity_vector proposal.

What follows is my first 5-min attempt.

Methods that kind of work out nicely

Methods that consume a single value.

Open question: Should these try to return the value on failure? That could look like this:

where std::out_of_bounds<optional<T>> to be an out-of-bounds exception that optionaly stores a T since we need to handle the case where moving the value in failed, and thus we couldn't move the value out. I don't think this is worth it: either the value was moved successfully, and is thus in the vector (so .back can be used to access it), or it wasn't moved successfully, in which case it is still in the value, so users can do:

 // don't do this:
vec.insert(pos, value_type{...});
// do this instead:
auto val = value_type{...};
vec.insert(pos, std::move(val));
// if insert throws, val wasn't moved from

So I think here we could get away with the following signatures as well:

Methods that consume a compile-time fixed number of values

If we try to support returning consumed temporaries here start to get out-of-hand and probably unusable without pattern matching and destructuring:

I would say that we should do the same as above: users that want to preserve the inputs in case of throw should store them somewhere first, and move them in afterwards:

Methods that consume a run-time number of values

Dereferencing an InputIterator potentially consumes a value from the range. Throwing out-of-bounds stops insertion, so the inserted elements are stored in the vector, and the rest are still in the range, this might work out:

For std::initializer_list I am not sure, but one cannot move out of an initializer list, so:

will just copy values out of it, so it should be fine.

Constructors

Constructors can't return expected. We can't replace them with static methods because then the fixed_capacity_vector cannot be constructed in-place, so they must be constructors:

We can just make them throw std::out_of_range for consistency with all other methods and be done with it.

gnzlbg commented 6 years ago

@caseycarter thoughts?

viboes commented 6 years ago

IMO this class should have as precondition that there is enough capacity, so no need here for expected.

expected could be appropriated for try_xxx functions, as you described.

Just my 2cts.

CaseyCarter commented 6 years ago

I agree with Vicente that it seems silly to muck about with the error design for functions with exceedingly simple preconditions like size() < capacity(). To satisfy everyone you will have to add functions that throw, and functions that return expected, and functions with UB. After doing so, LEWG will reject the design for being too complicated and fiddly. I suggest you stick with UB to allow implementations that prefer a different behavior to conformantly provide it. Callers who don't want exceptions can always check preconditions and use types with non-throwing constructors.

I would suggest that functions that accept rvalues like push_back provide a guarantee that the result state of the argument when an exception is thrown is whatever state it was left in by the throwing move constructor (variant wording may help here).

gnzlbg commented 5 years ago

Revision 2 used @CaseyCarter approach. The approach for revision 3 is to be clarified in #37 (it might be changed to std::abort instead of being UB).