recfactor: use FMT_THROW inside fmt.h to make code embedded-friendly

[burnpanck]

Currently, the library does not compile with exceptions disabled due to the various uses of throw inside the fmt.h header. The fmt library supports compilation with exceptions disabled by wrapping all potential exception raising points in the FMT_THROW macro which causes an assert instead when exceptions are disabled. To support that use-case, this PR uses FMT_THROW as-well in the fmt.h header.

[gitpod-io[bot]]

[burnpanck]

Ideally, the formatters in this library would behave exactly as the formatters of the standard library behave. I expect all major vendors of standard libraries to support compiling with exception disabled in some form or another, so they will have to find a way to deal with these errors. Unfortunately, the implementation of that behaviour will often be an implementation detail of the library (in a sense, FMT_THROW is too).

Another option would be to support exception-less in units directly. This would avoid the effort of managing implementation details of a number standard libraries. I personally don't like this that much in this case, given that units doesn't throw exceptions itself otherwise, and I am already patching/hacking into fmtlib to use a different error handler as theirs pulls std::fprintf which is not supported. This would make units another library to patch.

Maybe the most "complete" path would be to do support exception-less in units directly, but forward the failure handling to the respective formatting library in use where possible.

What would be your preferred approach?

[JohelEGP]

Others would benefit if this was part of https://github.com/fmtlib/fmt. And perhaps that has already been discussed there.

[burnpanck]

@JohelEGP It is already part of fmtlib: They never call throw directly, but use FMT_THROW instead. Their definition of FMT_THROW calls an assert if exceptions are disabled. But this units library here extends formatting with new formatters, which also have reasons to throw. The proposed change in this PR just re-uses the functionality from fmtlib instead of throwing directly. However, Mateusz rightfully points out that fmtlib is not the only formatting backend, given that their functionality has been integrated into the C++20 standard, and we might need a solution for the stdlib backend too.

[mpusz]

@burnpanck, did you see the CI errors for Windows MSVC 14.2? We cannot merge your changes as long as this fails.

[mpusz]

I expect all major vendors of standard libraries to support compiling with exception disabled in some form or another, so they will have to find a way to deal with these errors

I am not sure it that is the case as format header is not a part of freestanding (https://en.cppreference.com/w/cpp/freestanding).

[burnpanck]

Yeah, I saw the MSVC failures. The failure doesn't look MSVC specific though, so I suspect that is just the only test environment where exceptions are disabled. The root of the problem is that the FMT_THROW implementation for that case does not use fully-qualified namespaces (which it probably should). I suggest to simply move to our own UNITS_FMT_THROW. In that case, I would probably also change the other macros in units/bits/fmt_hacks.h to start with UNITS_FMT_ instead of just FMT_, to make it clear that these are our macros not theirs. Would you agree to that?

As for the scope of freestanding, as far as I understand freestanding is currently broken, and the reality of embedded development usually doesn't involve the -ffreestanding flag.

That said, given that I do not have the capacity now to research the way current standard libraries handle format exceptions when exceptions are disabled, why don't we just drop in our own exception alternative in the case of the standard library formatting for now, and leave that open for a future PR?

[burnpanck]

Yeah, the clang-format test in my pip-installed pre-commit fails with dyld[33347]: Library not loaded: /usr/local/opt/zstd/lib/libzstd.1.dylib which looks like a packaging issue relating to the arm64/x86_64 split on mac. I did run find src -iname "*.h" -o -iname "*.cpp" | xargs clang-format -i --style=file manually instead, but that doesn't change anything. Since the diff to master only contains ".h" and ".cpp" files, I don't know what the difference is compared to the pre-commit run. So if it isn't too much effort for you to do, that would be great.