[PATCH] D51103: [Support] Add a way to run a function on a detached thread

[JF Bastien via Phabricator via llvm-commits]

jfb added inline comments.


================
Comment at: include/llvm/Support/Threading.h:53
 
 /// llvm_execute_on_thread - Execute the given \p UserFn on a separate
 /// thread, passing it the provided \p UserData and waits for thread
----------------
Can you remove the `llvm_execute_on_thread` while you're here?


================
Comment at: lib/Support/Threading.cpp:119
+    llvm::Optional<unsigned> StackSizeInBytes) {
+  std::unique_ptr<AsyncThreadInfo> Info(new AsyncThreadInfo(std::move(Func)));
+  llvm_execute_on_thread_impl(&threadFuncAsync, Info.get(), StackSizeInBytes,
----------------
`llvm::make_unique` instead of `new`.


================
Comment at: lib/Support/Threading.cpp:122
+                              JoiningPolicy::Detach);
+  Info.release();
+}
----------------
I'd replace `get` above with `release`, and not have the `release` here, since you're kinda "moving" ownership into a `void*`.


================
Comment at: lib/Support/Unix/Threading.inc:65
+  if (::pthread_attr_init(&Attr) != 0) {
+    ReportErrnoFatal("pthread_attr_init failed");
+  }
----------------
Here and below, pthread doesn't set errno: it returns the error directly. You should pass the result in and call `strerror`.


https://reviews.llvm.org/D51103