Fix for: issue #471 Adds gsl::joining_thread (replaces original PR #509) #596
Conversation
Added -pthread flag to Clang build
| joining_thread() noexcept = default; | ||
|
|
||
| joining_thread(joining_thread const&) = delete; | ||
| joining_thread(joining_thread&& other) : t(std::move(other.t)) {} |
There was a problem hiding this comment.
Nothing forbids you to call joinable() on a moved from std::thread object.
|
I have to say, I'd feel much more comfortable with the idea of joining threads, if there was a way to indicate to the thread that termination was requested (something like a gsl:: |
| joining_thread() noexcept = default; | ||
|
|
||
| joining_thread(joining_thread const&) = delete; | ||
| joining_thread(joining_thread&& other) : t(std::move(other.t)) {} |
There was a problem hiding this comment.
I don't know if this PR is still active, but: this move constructor should be noexcept, or even better, defaulted.
| joining_thread(std::thread&& other) noexcept : t(std::move(other)) {} | ||
|
|
||
| joining_thread& operator=(joining_thread const&) = delete; | ||
| joining_thread& operator=(joining_thread&& other) noexcept |
| return *this; | ||
| } | ||
|
|
||
| joining_thread& operator=(std::thread const&) = delete; |
There was a problem hiding this comment.
This function signature should not exist (it shouldn't be explicitly =deleted).
| joining_thread(joining_thread&& other) : t(std::move(other.t)) {} | ||
|
|
||
| joining_thread(std::thread const&) = delete; | ||
| joining_thread(std::thread&& other) noexcept : t(std::move(other)) {} |
There was a problem hiding this comment.
I don't know if we're implementing a formal spec here, but if the design is still up for grabs, I'd be strongly inclined to make this constructor explicit — for the same reason that explicit unique_ptr<T>(T *p) is explicit.
If you do make it explicit, then you should remove the signature joining_thread& operator=(std::thread&& other) noexcept below, because we wouldn't want implicit assignment to work either, in that case.
|
|
||
| ~joining_thread() { if (t.joinable()) t.join(); } | ||
|
|
||
| bool joinable() const { return t.joinable(); } |
| void swap(joining_thread& t1, joining_thread& t2) noexcept | ||
| { | ||
| using std::swap; | ||
| swap(t1.t, t2.t); |
There was a problem hiding this comment.
It would be better to keep this as a hidden friend (defined in-line in the class body). Also, the implementation could be just t1.swap(t2); for simplicity.
This PR adds <gsl/gsl_thread> and tests/thread_tests.cpp
As per discussion in CppCoreGuidelines isocpp/CppCoreGuidelines#925
This PR provides
gsl::joining_thread(gsl::detached_thread was removed)(This PR is a cleaned up version of PR #509 - deleted)