folly: fix clang's -Wundefined-var-template

Summary:
[temp] (14)/6:
> A function template, member function of a class template, variable template, or static data member of a class template shall be defined in every translation unit in which it is implicitly instantiated (14.7.1) unless the corresponding specialization is explicitly instantiated (14.7.2) in some translation unit; no diagnostic is required.

`-Wundefined-var-template` warns on any implicit instantiations that are needed but could not be performed because the definition is not available. In particular, for valid code, this warns on templates/temploids which have their definition and all relevant explicit instantiations tucked away in some source file (but for which no explicit instantiation declarations are provided in the relevant header file) - used a few times in folly. This seems a bad style, the static data member template should either be defined in the header file or should be explicitly instantiated in each .cpp file.

Reviewed By: igorsugak

Differential Revision: D3270439

fbshipit-source-id: bf305fde3af575a265d65a0ae0bf95db5597587f

diff --git a/folly/Fingerprint.h b/folly/Fingerprint.h
index f1980ae215aa2b35afe248ad0e60b762fcad7b66..c7a20fb7536c9bdbac27f7382c957496988cd324 100644 (file)
--- a/folly/Fingerprint.h
+++ b/folly/Fingerprint.h
@@ -51,12 +51,20 @@
 namespace folly {
 
 namespace detail {
+
 template <int BITS>
 struct FingerprintTable {
-  static const uint64_t poly[1 + (BITS-1)/64];
-  static const uint64_t table[8][256][1 + (BITS-1)/64];
+  static const uint64_t poly[1 + (BITS - 1) / 64];
+  static const uint64_t table[8][256][1 + (BITS - 1) / 64];
 };
-}  // namespace detail
+
+template <int BITS>
+const uint64_t FingerprintTable<BITS>::poly[1 + (BITS - 1) / 64] = {};
+
+template <int BITS>
+const uint64_t FingerprintTable<BITS>::table[8][256][1 + (BITS - 1) / 64] = {};
+
+} // namespace detail
 
 /**
  * Compute the Rabin fingerprint.

diff --git a/folly/SharedMutex.cpp b/folly/SharedMutex.cpp
index e7c5267e884fd8c4c8c345fba916c40731a89542..23266b7c1b22bd1464060474d8aa52436370b997 100644 (file)
--- a/folly/SharedMutex.cpp
+++ b/folly/SharedMutex.cpp
@@ -16,7 +16,8 @@
 
 #include "SharedMutex.h"
 
-COMMON_CONCURRENCY_SHARED_MUTEX_DECLARE_STATIC_STORAGE(
-    folly::SharedMutexReadPriority);
-COMMON_CONCURRENCY_SHARED_MUTEX_DECLARE_STATIC_STORAGE(
-    folly::SharedMutexWritePriority);
+namespace folly {
+// Explicitly instantiate SharedMutex here:
+template class SharedMutexImpl<true>;
+template class SharedMutexImpl<false>;
+}

diff --git a/folly/SharedMutex.h b/folly/SharedMutex.h
index 15dc91e6b589378ebb6d20991106156000bfe955..2d71a88473b0f612508181b5172230ef1c13f676 100644 (file)
--- a/folly/SharedMutex.h
+++ b/folly/SharedMutex.h
@@ -1429,16 +1429,33 @@ class SharedMutexImpl {
   }
 };
 
-#define COMMON_CONCURRENCY_SHARED_MUTEX_DECLARE_STATIC_STORAGE(type) \
-  template <>                                                        \
-  type::DeferredReaderSlot                                           \
-      type::deferredReaders[type::kMaxDeferredReaders *              \
-                            type::kDeferredSeparationFactor] = {};   \
-  template <>                                                        \
-  FOLLY_TLS uint32_t type::tls_lastTokenlessSlot = 0;
+template <
+    bool ReaderPriority,
+    typename Tag_,
+    template <typename> class Atom,
+    bool BlockImmediately>
+typename SharedMutexImpl<ReaderPriority, Tag_, Atom, BlockImmediately>::
+    DeferredReaderSlot
+        SharedMutexImpl<ReaderPriority, Tag_, Atom, BlockImmediately>::
+            deferredReaders[kMaxDeferredReaders * kDeferredSeparationFactor] =
+                {};
+
+template <
+    bool ReaderPriority,
+    typename Tag_,
+    template <typename> class Atom,
+    bool BlockImmediately>
+FOLLY_TLS uint32_t
+    SharedMutexImpl<ReaderPriority, Tag_, Atom, BlockImmediately>::
+        tls_lastTokenlessSlot = 0;
 
 typedef SharedMutexImpl<true> SharedMutexReadPriority;
 typedef SharedMutexImpl<false> SharedMutexWritePriority;
 typedef SharedMutexWritePriority SharedMutex;
 
+// Prevent the compiler from instantiating these in other translation units.
+// They are instantiated once in SharedMutex.cpp
+extern template class SharedMutexImpl<true>;
+extern template class SharedMutexImpl<false>;
+
 } // namespace folly

diff --git a/folly/detail/CacheLocality.cpp b/folly/detail/CacheLocality.cpp
index e75120d6b39c2aa64863ef46a4304461ba4a96d0..c62e0ac8619cda5ce9d250f808319457b1dbc308 100644 (file)
--- a/folly/detail/CacheLocality.cpp
+++ b/folly/detail/CacheLocality.cpp
@@ -28,8 +28,6 @@
 #include <folly/Format.h>
 #include <folly/ScopeGuard.h>
 
-DECLARE_ACCESS_SPREADER_TYPE(std::atomic)
-
 namespace folly {
 namespace detail {
 
@@ -233,21 +231,11 @@ Getcpu::Func Getcpu::resolveVdsoFunc() {
 
 #ifdef FOLLY_TLS
 /////////////// SequentialThreadId
-
-template <>
-std::atomic<size_t> SequentialThreadId<std::atomic>::prevId(0);
-
-template <>
-FOLLY_TLS size_t SequentialThreadId<std::atomic>::currentId(0);
+template struct SequentialThreadId<std::atomic>;
 #endif
 
 /////////////// AccessSpreader
-
-template <>
-Getcpu::Func AccessSpreader<std::atomic>::pickGetcpuFunc() {
-  auto best = Getcpu::resolveVdsoFunc();
-  return best ? best : &FallbackGetcpuType::getcpu;
-}
+template struct AccessSpreader<std::atomic>;
 
 } // namespace detail
 } // namespace folly

diff --git a/folly/detail/CacheLocality.h b/folly/detail/CacheLocality.h
index 1ede43cbef70b4f60d4f06f40a77890be419dfa1..c9baf5281ac35afb97fd1aea184162a0ede740d0 100644 (file)
--- a/folly/detail/CacheLocality.h
+++ b/folly/detail/CacheLocality.h
@@ -161,6 +161,16 @@ struct SequentialThreadId {
 
   static FOLLY_TLS size_t currentId;
 };
+
+template <template <typename> class Atom>
+Atom<size_t> SequentialThreadId<Atom>::prevId(0);
+
+template <template <typename> class Atom>
+FOLLY_TLS size_t SequentialThreadId<Atom>::currentId(0);
+
+// Suppress this instantiation in other translation units. It is
+// instantiated in CacheLocality.cpp
+extern template struct SequentialThreadId<std::atomic>;
 #endif
 
 struct HashingThreadId {
@@ -277,7 +287,10 @@ struct AccessSpreader {
   static bool initialized;
 
   /// Returns the best getcpu implementation for Atom
-  static Getcpu::Func pickGetcpuFunc();
+  static Getcpu::Func pickGetcpuFunc() {
+    auto best = Getcpu::resolveVdsoFunc();
+    return best ? best : &FallbackGetcpuType::getcpu;
+  }
 
   /// Always claims to be on CPU zero, node zero
   static int degenerateGetcpu(unsigned* cpu, unsigned* node, void*) {
@@ -326,22 +339,20 @@ struct AccessSpreader {
   }
 };
 
-template <>
-Getcpu::Func AccessSpreader<std::atomic>::pickGetcpuFunc();
-
-#define DECLARE_ACCESS_SPREADER_TYPE(Atom)                                     \
-  namespace folly {                                                            \
-  namespace detail {                                                           \
-  template <>                                                                  \
-  Getcpu::Func AccessSpreader<Atom>::getcpuFunc =                              \
-      AccessSpreader<Atom>::degenerateGetcpu;                                  \
-  template <>                                                                  \
-  typename AccessSpreader<Atom>::CompactStripe                                 \
-      AccessSpreader<Atom>::widthAndCpuToStripe[129][128] = {};                \
-  template <>                                                                  \
-  bool AccessSpreader<Atom>::initialized = AccessSpreader<Atom>::initialize(); \
-  }                                                                            \
-  }
+template <template <typename> class Atom>
+Getcpu::Func AccessSpreader<Atom>::getcpuFunc =
+    AccessSpreader<Atom>::degenerateGetcpu;
+
+template <template <typename> class Atom>
+typename AccessSpreader<Atom>::CompactStripe
+    AccessSpreader<Atom>::widthAndCpuToStripe[kMaxCpus + 1][kMaxCpus] = {};
+
+template <template <typename> class Atom>
+bool AccessSpreader<Atom>::initialized = AccessSpreader<Atom>::initialize();
+
+// Suppress this instantiation in other translation units. It is
+// instantiated in CacheLocality.cpp
+extern template struct AccessSpreader<std::atomic>;
 
 } // namespace detail
 } // namespace folly

diff --git a/folly/test/CacheLocalityBenchmark.cpp b/folly/test/CacheLocalityBenchmark.cpp
index 4815cc24cac3be7e9affb0cca99a1284db1ef115..81f094d95111e78dfff894acbd323b611c9ff6ba 100644 (file)
--- a/folly/test/CacheLocalityBenchmark.cpp
+++ b/folly/test/CacheLocalityBenchmark.cpp
@@ -31,7 +31,6 @@ using namespace folly::detail;
   template <typename dummy>                            \
   struct tag {};                                       \
   }                                                    \
-  DECLARE_ACCESS_SPREADER_TYPE(tag)                    \
   namespace folly {                                    \
   namespace detail {                                   \
   template <>                                          \

diff --git a/folly/test/CacheLocalityTest.cpp b/folly/test/CacheLocalityTest.cpp
index 9c326fde4ba320b7f88b198ee8fc5ac8d695da8b..33af739bd626c916702e881310553b596f11c292 100644 (file)
--- a/folly/test/CacheLocalityTest.cpp
+++ b/folly/test/CacheLocalityTest.cpp
@@ -413,7 +413,6 @@ TEST(AccessSpreader, Simple) {
   template <typename dummy>                            \
   struct tag {};                                       \
   }                                                    \
-  DECLARE_ACCESS_SPREADER_TYPE(tag)                    \
   namespace folly {                                    \
   namespace detail {                                   \
   template <>                                          \

diff --git a/folly/test/DeterministicSchedule.cpp b/folly/test/DeterministicSchedule.cpp
index fe59cc02145c47357fb68158a16914119a9cb634..f7668fc587e5bd68d9270683f3b2e0c8007bafc8 100644 (file)
--- a/folly/test/DeterministicSchedule.cpp
+++ b/folly/test/DeterministicSchedule.cpp
@@ -15,15 +15,17 @@
  */
 
 #include <folly/test/DeterministicSchedule.h>
+
+#include <assert.h>
+
 #include <algorithm>
 #include <list>
 #include <mutex>
 #include <random>
-#include <utility>
 #include <unordered_map>
-#include <assert.h>
+#include <utility>
 
-DECLARE_ACCESS_SPREADER_TYPE(folly::test::DeterministicAtomic)
+#include <folly/Random.h>
 
 namespace folly {
 namespace test {
@@ -139,7 +141,7 @@ int DeterministicSchedule::getRandNumber(int n) {
   if (tls_sched) {
     return tls_sched->scheduler_(n);
   }
-  return std::rand() % n;
+  return Random::rand32() % n;
 }
 
 int DeterministicSchedule::getcpu(unsigned* cpu,

diff --git a/folly/test/SharedMutexTest.cpp b/folly/test/SharedMutexTest.cpp
index 8672e665d1ba54412c4c8547e84d37f8824d9b12..98df9d09cbde1fe0f7da6248d03511fcf6348e3f 100644 (file)
--- a/folly/test/SharedMutexTest.cpp
+++ b/folly/test/SharedMutexTest.cpp
@@ -42,11 +42,6 @@ typedef SharedMutexImpl<true, void, DeterministicAtomic, true>
 typedef SharedMutexImpl<false, void, DeterministicAtomic, true>
     DSharedMutexWritePriority;
 
-COMMON_CONCURRENCY_SHARED_MUTEX_DECLARE_STATIC_STORAGE(
-    DSharedMutexReadPriority);
-COMMON_CONCURRENCY_SHARED_MUTEX_DECLARE_STATIC_STORAGE(
-    DSharedMutexWritePriority);
-
 template <typename Lock>
 void runBasicTest() {
   Lock lock;