boostorg / boostorg/container

Bring back std exceptions (by default)

Open
#183 11 comments 1 reaction 0 assignees View on GitHub
Dominant language
C++
Stars
127
Forks
126
Avg merge
18h 53m
Merged PRs (30d)
1

Description

https://github.com/boostorg/container/commit/d5a830434eb68b9fc65cfbc99572a96e4ee0f70a replace the usage of std:: exceptions by new, custom ones which resulted in quite a few critics, e.g. on reddit: https://www.reddit.com/r/cpp/comments/mskpfp/boost_c_libraries_v1760

I have to agree and would suggest to revert that.

I think that is an example of optimization guided by microbenchmarks which usually fail in practice. I guess the size savings from the preprocessed lines come from avoiding std::string (transitively) but binary size will increase, as stdexcept will very likely be included somewhere (already or later). So my point is that this optimizations very likely pessimizes usual usecases.

Also there are implementation issues:
- Needless `#ifdef BOOST_CONTAINER_USE_STD_EXCEPTIONS` checks in the `throw_*` functions
- Likely visibility issues when throwing/catching such exceptions over DSO boundaries due to `-fvisibility=hidden` being the default (now) and no BOOST_SYMBOL_EXPORT or similar on the class
- docs were forgotten to be updated: `

  • If BOOST_NO_EXCEPTIONS is NOT defined std::logic_error(str) is thrown.
  • `
    - Possibly silently breaking user code which needs to adapt to these changes

    So IMO the easiest is be to revert that. Or at least don't make it the default so it is opt-in for users who want this optimization.

    Note also that this may lead to ODR violations if one dependency defines it to use std exceptions and another does not. That is another reason to not do this.

    Contributor guide

    No contributing guide indexed for this repository

    Assessment

    This issue has not been assessed yet.

    Get new issues in your inbox

    A short digest of beginner-friendly GitHub issues.