Add StartThread type checking wrapper #8303
Conversation
include/rocksdb/env.h
Outdated
| using FWType = FunctorWrapper<Args...>; | ||
| StartThread( | ||
| [](void* arg) { | ||
| auto* functor = reinterpret_cast<FWType*>(arg); |
There was a problem hiding this comment.
Do not use reinterpret_cast to cast from void* (in new code)
include/rocksdb/env.h
Outdated
| // Start a new thread, invoking "function(args...)" within the new thread. | ||
| // When "function(args...)" returns, the thread will be destroyed. | ||
| template <typename FunctionT, typename... Args> | ||
| auto WrapStartThread(FunctionT function, Args&&... args) -> |
There was a problem hiding this comment.
Why not just void return type without all the type trait stuff? The definition is inline and always will be because it's such a general template, so bad instantiations will fail to compile based on the (small) function definition being ill-formed.
There was a problem hiding this comment.
I agree, but I thought the error message would be a little nicer if the compiler failed to substitute function in the template at the entry to the WrapStartThread (or StartThreadTypeSafe) than if it failed to substitute a function in the constructor of std::function. The only difference is the size of the error. If it's not that important, I'll delete it, of course:)
include/rocksdb/env.h
Outdated
| // Start a new thread, invoking "function(args...)" within the new thread. | ||
| // When "function(args...)" returns, the thread will be destroyed. | ||
| template <typename FunctionT, typename... Args> | ||
| auto WrapStartThread(FunctionT function, Args&&... args) -> |
There was a problem hiding this comment.
Being "wrapped" is an implementation detail. The key feature vs. StartThread is type safety. Perhaps StartThreadTyped
|
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@pdillinger merged this pull request in 748e3ac. |
FunctorWrapperto invoke the function with given parametersis_invocabletype traitWrapStartThreadwhich wrapsStartThreadwith type checking coverWrapStartThreadin testutil/thread_local_test.cc#8285