We don't catch this specific exception, so no behavioural change, but we
do no longer violate the implicit noexcept for destructors in C++11.
(A quite similar commit is present in one of Itms' branches for VS2015 support.)
(GCC6 warning)
Details
Details
- Reviewers
Itms - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP19612: Call std::terminate() instead of throwing in a destructor.
Read the standard (or the docs), possibly that warning, check that we never even try to catch that warning, decide that just terminating explicitly is better than doing so via itricate ways.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/987/ for more details.
Comment Actions
Isn't it better to call std::abort() instead of std::terminate()? In the standard it seems that std::terminate() is only discussed in relation to exceptions, and it doesn't seem like it is a function one would call normally.
Comment Actions
The std::terminate function never throws exceptions. Also std::terminate allows to specify itself handlers, but std::abort doesn't. Also I've heard that std::abort could throw exceptions (lib dependency).
Comment Actions
Well the default handler just calls std::abort, one could specify another one if one really wanted. That said I mostly picked std::terminate due to the warning message (and them being mostly equivalent).
The std::terminate function never throws exceptions. Also std::terminate allows to specify itself handlers, but std::abort doesn't. Also I've heard that std::abort could throw exceptions (lib dependency).
I'd be very surprised if std::abort could even throw an exception, given that it is called by the default handler (or is it) for std::terminate (which is both noreturn and noexcept), and it's just abort(3), so it cannot throw something (at least not in the sense that it would return to any code we provided.
Comment Actions
This looks good to me, and builds on VS2013 and VS2015 (fixing the warning in the last case).
In my branch I used DEBUG_WARN_ERR(ERR::EXCEPTION); for consistency with line 608. Could this have the benefit of not being as silent as terminate()?
Comment Actions
Could this have the benefit of not being as silent as terminate()?
I'd expect such a failure to be quite easy to reproduce in any case, also I don't think I've seen that specific piece of code fail so far.