Static Analysis #546
Merged
Static Analysis #546
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Transfering work in progress on handling broken promise exceptions between machines.
Changed the await() and await_for interface to take an rvalue references. Use `move()` or `copy()` when calling with an lvalue. (This change generated a lot of test changes!). Fixes to future coroutine support to correctly report exceptions. Check box items should be in a subsequent PR [] Support waiting for multiple futures directly.
Fixed up remaining clang-tidy issues. Added (back) await and await_for with implicit copy as deprecated APIs.
|
Just reading the summary it seems like a lot of changes that could've been separate PR's, for example, a clang tidy pass seems like a different topic from fixing the bug. |
Unfortunately, it doesn't work in .hpp files :-(
Yes - it should have been about 5 PRs. It started with "could clang-tidy find the same issue as Coverity" (answer, not that I've been able to determine) - and snowballed. |
|
FYI I ran clang-tidy locally over these changes and it seems to work. The presets are nice. Haven't had time yet to review the changes themselves, though. |
|
Going to take the heat for pushing my own oversized PR... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR was started to address an issue reported by Covarity regarding the exception handling in the future reduction code. It led to a rewrite of the future reduction code, the inclusion of clang-tidy and clang-tidy fixes in the CMake setup, and several other changes:
API Changes
package()will now reduce futures (future<future> -> future). All APIs that return futures now reduce the future.await()andawait_for()take thefutureby rvalue-ref. They now take it by rvalue-ref with deprecated overloads for const-ref for compatibility. I'm moving away from implicit copies. Usestlab::copy()orstd::move()as appropriate to upgrade to the new interface.[[nodiscard]]to many interfaces. In general, these should catch misuse.get_ready()API tofuturethat can be used in a recover() clause or when the future is known to be ready. It asserts the future is ready and would be a race to invoke on a non-ready future.Fixes
task(const task&&)is now deleted, which avoids a bad recursive runtime error if you attempt to copy a task.future<>with C++20 coroutines, exceptions are now correctly reported.forestis now atomic. Aconst forestcan be safely shared across threads.Improvements
cert-*,performance-*, andmodernize*-checks for C++17.