simdjson / simdjson/simdjson

Exception handling inside tag_invoke customizations

Open
#2,858 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

help wanted
Dominant language
C++
Stars
24.3k
Forks
1.3k
Avg merge
1d 3h
Merged PRs (30d)
16

Description

As far as I can tell you are allowed to throw exceptions inside tag invoke customizations to propagate errors instead of forwarding error codes. At least the appendable container customization only removes default constructed values in case errors get propagated via the error code.

template <concepts::appendable_containers T, typename ValT>
error_code tag_invoke(deserialize_tag, ValT &val, T &out) noexcept(false) {
  using value_type = typename std::remove_cvref_t<T>::value_type;
  static_assert(
      deserializable<value_type, ValT>,
      "The specified type inside the container must itself be deserializable");
  static_assert(
      std::is_default_constructible_v<value_type>,
      "The specified type inside the container must default constructible.");
  arm64::ondemand::array arr;
  if constexpr (std::is_same_v<std::remove_cvref_t<ValT>, arm64::ondemand::array>) {
    arr = val;
  } else {
    SIMDJSON_TRY(val.get_array().get(arr));
  }

  for (auto v : arr) {
    if constexpr (concepts::returns_reference<T>) {
      if (auto const err = v.get<value_type>().get(concepts::emplace_one(out));
          err) {
        // If an error occurs, the empty element that we just inserted gets
        // removed. We're not using a temp variable because if T is a heavy
        // type, we want the valid path to be the fast path and the slow path be
        // the path that has errors in it.
        if constexpr (requires { out.pop_back(); }) {
          static_cast<void>(out.pop_back());
        }
        return err;
      }
    } else {
      value_type temp;
      if (auto const err = v.get<value_type>().get(temp); err) {
        return err;
      }
      concepts::emplace_one(out, std::move(temp));
    }
  }
  return SUCCESS;
}

Is this an accepted limitation of customizations or something that should somehow be changed?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the appendable-container deserialize_tag customization shown in the issue and how tag_invoke callers handle exceptions versus error codes. Check existing customization and exception-handling tests, if present; done means the accepted behavior for exceptions is established and the container state or documented limitation is handled consistently.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend-api-design
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.