[PATCH] D53424: Enable thread specific cl::opt values for multi-threaded support

Yevgeny Rouban via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 24 04:53:34 PDT 2018


yrouban added inline comments.


================
Comment at: include/llvm/Support/CommandLine.h:1184
+struct Destructible {
+  virtual ~Destructible() = default;
+};
----------------
nhaehnle wrote:
> This should be declared explicitly in a .cpp file to provide an "anchor" for the vtable. (Otherwise the destructor and the vtable etc. are emitted in every compilation unit that includes this file, which is a waste of compile time.)
ok


================
Comment at: unittests/Support/CommandLineTest.cpp:25
 
+#ifdef HAVE_PTHREAD_H
+#include <chrono>
----------------
delcypher wrote:
> This unit test needs to work on Windows. I see no good reason to use pthread here. Can't you just use `std::thread` instead?
> Also guarding `chrono` and `thread` headers on pthread being available seems wrong given that those headers will be available on platforms without pthreads.
ok


Repository:
  rL LLVM

https://reviews.llvm.org/D53424





More information about the llvm-commits mailing list