Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch exceptions by _const_ reference? #518

Closed
villintehaspam opened this issue Feb 2, 2016 · 4 comments
Closed

Catch exceptions by _const_ reference? #518

villintehaspam opened this issue Feb 2, 2016 · 4 comments
Assignees

Comments

@villintehaspam
Copy link

The rule in E.15 is to catch exceptions from a hierarchy by reference to avoid slicing, and the examples use a non-const reference.

The common advice that I have seen so far is to catch exceptions by const reference, IIRC this has mostly been because of a general desire for const correctness. In his answer to this StackOverflow question, Howard Hinnant does provide some additional rationale for using const references though. The rationale being the risk of simultaneous handling of the same exception in multiple threads coming out of a shared_future, which seems to have been a topic for discussion in the LWG.

Should E.15 recommend using const references to exceptions rather than just references? It seems like an easy way to avoid potential multithreading issues, with little* cost.

*: Well, for my typical use cases anyway. People doing more advanced/weird stuff with their caught exceptions are probably either already aware of what they are doing and would keep doing it the way they do it regardless of what the guideline says, or they are unaware of what they are doing and should be told of a sensible default guideline (when/if they eventually start reading those).

@LegalizeAdulthood
Copy link

The Boost.Exception library has a discussion of why you might want to catch by non-const reference. Basically the idea is that an inner scope may originate the exception and an outer scope adds more context to the exception before rethrowing it up the call chain to the error handling site where the additional context is used to more properly inform the user of the details of the error.

I think it is reasonable to mention use of const reference when you aren't modifying the exception object, but this is just back to being a good citizen with respect to const and not specific to exceptions.

@villintehaspam
Copy link
Author

@LegalizeAdulthood: Yes, adding more information to the object and then rethrowing seems to be the use case for not catching by const ref.

However, as Howard hints at, this is not a safe thing to do in general unless you know that the exception is not handled in multiple threads or that you are handling the exception object without data races.

Even that would be suspicious to me - for instance boost::exception will overwrite the error information for a tag if written again. So even if boost::exception would be thread safe, which I don't think it is, you can still get into trouble. If one thread adds a piece of information to the exception object, say the name of the calling function, then another thread could overwrite it. When an exception handler further up the chain tries to read the information, it might not be correct for that thread anymore.

So it kind of falls back to that you either

  1. shouldn't modify the exception object at all, in which case you might as well catch it by const ref
  2. or you must ensure to use it in a thread safe manner (both race free and "logically" thread safe), which may cost as much as just copying the relevant information into a new object
  3. or you must ensure that you are never accidentally handle the exception in multiple threads at the same time.

For a simple default guideline, it would seem much easier to just recommend to catch using const ref, would you agree?

My personal opinion would be that the added const also contributes to readability - just like a function parameter taken by non-const reference screams "this parameter will be changed", so does catching by non-const reference to me. That seems to be more subjective though I guess.

@AndrewPardoe
Copy link
Contributor

Per our editors' discussion, we can add a reference to con.1, noting that most exceptions can and thus should be caught by const reference.

@BjarneStroustrup
Copy link
Contributor

Comment about const added

hjmjohnson added a commit to BRAINSia/BRAINSTools that referenced this issue May 11, 2022
Following C++ Core Guidelines, April 10, 2022, which says that it is "typically
better" to catch by const reference than to catch by non-const reference. At
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e15-throw-by-value-catch-exceptions-from-a-hierarchy-by-reference

Discussed at C++ Core Guidelines issue isocpp/CppCoreGuidelines#518 "Catch
exceptions by const reference?", from February 3, 2016.

find . -type f | egrep '\.[it]?(cc|hh|[ch](\+\+|pp?|xx)?)$' | fgrep -v ThirdParty |     xargs sed -r -i         -e 's/catch \((const )?/catch (const /g'         -e 's/catch \(const \.\.\.\)/catch (...)/g'
hjmjohnson added a commit to ANTsX/ANTs that referenced this issue May 12, 2022
Following C++ Core Guidelines, April 10, 2022, which says that it is "typically
better" to catch by const reference than to catch by non-const reference. At
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#e15-throw-by-value-catch-exceptions-from-a-hierarchy-by-reference

Discussed at C++ Core Guidelines issue isocpp/CppCoreGuidelines#518 "Catch
exceptions by const reference?", from February 3, 2016.

find . -type f | egrep '\.[it]?(cc|hh|[ch](\+\+|pp?|xx)?)$' | fgrep -v ThirdParty |     xargs sed -r -i         -e 's/catch \((const )?/catch (const /g'         -e 's/catch \(const \.\.\.\)/catch (...)/g'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants