Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
gnzlbg opened this issue Feb 14, 2018 · 4 comments
Closed

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

gnzlbg opened this issue Feb 14, 2018 · 4 comments

Comments

@gnzlbg
Copy link
Owner

gnzlbg commented Feb 14, 2018

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

  • [[nodiscard]] constexpr expected<void, std::out_of_bounds> push_back(const value_type& x);
  • [[nodiscard]] constexpr expected<iterator, std::out_of_bounds> insert(const_iterator position, const value_type& x);
  • [[nodiscard]] constexpr expected<iterator, std::out_of_bounds> insert(const_iterator position, size_type n, const value_type& x);

Methods that consume a single value.

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

  • [[nodiscard]] constexpr expected<void, std::out_of_bounds<optional<value_type>>> push_back(value_type&& x);

  • constexpr expected<iterator, std::out_of_bounds<optional<value_type>>> insert(const_iterator position, value_type&& x);

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:

  • [[nodiscard]] constexpr expected<void, std::out_of_bounds> push_back(value_type&& x);

  • [[nodiscard]] constexpr expected<iterator, std::out_of_bounds> insert(const_iterator position, value_type&& x);

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:

  • template <class... Args> [[nodiscard]] constexpr expected<iterator, out_of_bounds<tuple<optional<Args...>>>> emplace(const_iterator position, Args&&... args);

  • template <class... Args> [[nodiscard]] constexpr expected<reference, out_of_bounds<tuple<optional<Args...>>>> emplace_back(Args&&... args);

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:

  • template <class... Args> [[nodiscard]] constexpr expected<iterator, out_of_bounds> emplace(const_iterator position, Args&&... args);

  • template <class... Args> [[nodiscard]] constexpr expected<reference, out_of_bounds> emplace_back(Args&&... args);

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:

  • template <class InputIterator> [[nodiscard]] constexpr expected<iterator, std::out_of_bounds> insert(const_iterator position, InputIterator first, InputIterator last);

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

  • [[nodiscard]] constexpr expected<iterator, std::out_of_bounds> insert(const_iterator position, initializer_list<value_type> il);

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:

  • fixed_capacity_vector(value_type, size), fixed_capacity_vector(begin, end)...

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

@gnzlbg gnzlbg changed the title Error handling for rN: expected<T, std::some_exception> Error handling using expected<T, std::some_exception> Feb 14, 2018
@gnzlbg
Copy link
Owner Author

gnzlbg commented Feb 14, 2018

@CaseyCarter thoughts?

@viboes
Copy link

viboes commented Feb 14, 2018

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
Copy link

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
Copy link
Owner Author

gnzlbg commented Nov 12, 2018

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).

@gnzlbg gnzlbg closed this as completed Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants