sparse-map
sparse-map copied to clipboard
Add support for platforms without exceptions
Also allows controlling the use of exceptions on platforms with exceptions.
Fixes #18
Hi,
Thanks for your PR. It would effectively be nice to support fno-exceptions, I didn't took the time to do it yet for this library.
Could you check out how it was done for the other tsl
projects (like in tsl::robin_map
) for example) and check if it can eventually be done the same way in the PR for coherence?
Thanks
I've pushed the following changes:
-
TSL_ENABLE_EXCEPTIONS
->TSL_NO_EXCEPTIONS
. - Detection of the default even without CMake.
- Changed the
throw_
function to a macro.
I think these are better in this PR as compared to the code in robin_map
:
-
std::abort
is used instead ofstd::terminate
, becausestd::terminate
is not available on some platform without exceptions. -
fprintf
instead ofcerr
avoids the heavyiostreams
dependency. - Always prints the error message before aborting. The other version does so only if
TSL_DEBUG
is defined.
Thank you for the update. A few comments.
-
Could you use the
SH
(sparse hash) liketsl_sh_assert
instead of `SM´? -
std::abort is used instead of std::terminate, because std::terminate is not available on some platform without exceptions.
Yes, we can use std::abort
instead but could you then rename TSL_SM_THROW_OR_TERMINATE
to TSL_SH_THROW_OR_ABORT
-
fprintf instead of cerr avoids the heavy iostreams dependency.
Could you only print when TSL_DEBUG
is enabled? I would like to avoid importing cstdio
and printing to stderr by default.
-
I would prefer to avoid having an extra
exceptions.h
file if possible, can't we have everything defined insparse_growth_policy.h
? I know it is a bit confusing to add the functionality there compared to the name of the file but we need these to be defined before the exceptions usage insparse_growth_policy.h
. -
Is the CMake detection really useful compared to the detection in the header? Is there really a case where someone would want to use
tsl::sparse_map
with exceptions enabled while they disable them for them? -
Could you also add a config in the CI with exceptions disabled? Like https://github.com/Tessil/robin-map/blob/master/.github/workflows/ci.yml#L17
Thanks
I modified a bit your PR to address some of my comments, mainly moving things to sparse_growth_policy.h
to avoid a new file, renaming to TSL_SH_*
prefix and adapting the test to run an extra -fno-exceptions
CI. Hope it is alright for you.
I will merge the PR, thanks.
Thanks for following up on my PR!