[SERVER-69882] Audit usage of `throw()` in third party libraries Created: 21/Sep/22 Updated: 03/Nov/22 Resolved: 20/Oct/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Celina Tala | Assignee: | Andrew Morrow (Inactive) |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Participants: | |||||||||
| Description |
|
SERVER-42810 replaced usage of `throw()` in the codebase, but there's still some usage in third-party libraries. We should audit the usage before deciding whether we should remove it. |
| Comments |
| Comment by Andrew Morrow (Inactive) [ 20/Oct/22 ] |
|
There is no action for us to take here. With the v3 toolchain and C++17, it is not an error to write an empty throw(), it just becomes equivalent to writing noexcept. With the v4 toolchain in C++20 mode, it is technically no longer supported to even write throw(), however, none of the v4 toolchains appear to reject it, nor does VS 2022, because if any of them did one or more of our C++20 canary builders on the waterfall would be failing. It also does not appear that any of our toolchains offer an opt-in diagnostic that would flag uses of throw() as warnings that we could upgrade to errors, so we currently have no way to prevent future additions of throw(). Meanwhile, if there were any problematic throw() statements latent in our code we would just fix them if/when one of clang, gcc, or MSVC started rejecting them in C++20 mode. That would also mean that we would need to address any in our third_party trees as well, but so would the upstream maintainers (assuming they bother to stay current with stable compilers). Either way, we would address the issue by either pulling a new version from upstream, or making the change ourselves in our forks repo, sending a PR, and merging the fix back to our vendored copy. Basically, I don't think any audit is required and we can deal with it if it ever becomes an issue in practice. |
| Comment by Billy Donahue [ 21/Sep/22 ] |
|
Dynamic exception specifiers like `throw()` were removed in C++17. The description "Remove `throw()` from third party libraries" may be stating the requested action too simply. We are not interested in wiping out the appearances of `throw()` entirely. Most or all of these `throw()` occurrences are for backward compatibility with pre-C++11 builds and protected by ifdefs. Regarding the suggestion about weak references: This is a language feature (called an "exception specification"), not a function call. It's not something that a library trick can fix. |
| Comment by Daniel Moody [ 21/Sep/22 ] |
|
well need to track these in the forks, so updates are easier. Alternatively could we using some kind of weak symbol reference to overwrite the symbol at link time? |