glaze icon indicating copy to clipboard operation
glaze copied to clipboard

from json (at least) operations should not be noexcept

Open sjanel opened this issue 1 year ago • 1 comments

These functions currently marked noexcept (also in the documentation)

      template <string_t T>
      struct from<JSON, T>
      {
         template <auto Opts, class It, class End>
            requires(has_is_padded(Opts))
         static void op(auto& value, is_context auto&& ctx, It&& it, End&& end) noexcept  // <- should not be noexcept

cannot be noexcept as they can allocate memory.

sjanel avatar Oct 02 '24 18:10 sjanel

This brings up a good point, but is a bit more nuanced. If the program is compiled with no exceptions then indeed those allocations won't throw, but rather abort (i.e. panic).

So, it can be true for them to be noexcept when compiled with -fno-except. The noexcept should actually be conditional on the compiler's flag. I'll make this change.

Currently, this will convert the behavior into aborting when a throw would occur, which gives the behavior of -fno-except for this function when exceptions are enabled. But, I think this is bad, as users would expect exceptions to be catchable.

Now, I don't like requiring users to both use the error code handling and have try-catch blocks. I tend to think Glaze should catch these runtime exceptions and convert them into error codes so that the error code interface can handle all errors.

stephenberry avatar Oct 02 '24 23:10 stephenberry

I've decided that for the sake of simplicity I should simply remove noexcept from locations that could throw. It just makes development simpler and compilers tend to be smart when handling code that doesn't throw. Since these are all functions and not constructors (where noexcept is critical for efficient moves) I don't see any significant downsides.

I'm culling a lot of noexcept specifiers in #1420

stephenberry avatar Oct 31 '24 13:10 stephenberry