Reverted commit D3270439

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: e8cdb0452c2f6240a348b93cdbf9975813f27bfe

diff --git a/folly/Fingerprint.h b/folly/Fingerprint.h
index c7a20fb7536c9bdbac27f7382c957496988cd324..f1980ae215aa2b35afe248ad0e60b762fcad7b66 100644 (file)
--- a/folly/Fingerprint.h
+++ b/folly/Fingerprint.h
@@ -51,20 +51,12 @@
 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];
 };
-
-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
+}  // namespace detail
 
 /**
  * Compute the Rabin fingerprint.

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

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

diff --git a/folly/detail/CacheLocality.h b/folly/detail/CacheLocality.h
index c9baf5281ac35afb97fd1aea184162a0ede740d0..1ede43cbef70b4f60d4f06f40a77890be419dfa1 100644 (file)
--- a/folly/detail/CacheLocality.h
+++ b/folly/detail/CacheLocality.h
@@ -161,16 +161,6 @@ 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 {
@@ -287,10 +277,7 @@ struct AccessSpreader {
   static bool initialized;
 
   /// Returns the best getcpu implementation for Atom
-  static Getcpu::Func pickGetcpuFunc() {
-    auto best = Getcpu::resolveVdsoFunc();
-    return best ? best : &FallbackGetcpuType::getcpu;
-  }
+  static Getcpu::Func pickGetcpuFunc();
 
   /// Always claims to be on CPU zero, node zero
   static int degenerateGetcpu(unsigned* cpu, unsigned* node, void*) {
@@ -339,20 +326,22 @@ struct AccessSpreader {
   }
 };
 
-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>;
+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(); \
+  }                                                                            \
+  }
 
 } // namespace detail
 } // namespace folly

diff --git a/folly/test/CacheLocalityBenchmark.cpp b/folly/test/CacheLocalityBenchmark.cpp
index 81f094d95111e78dfff894acbd323b611c9ff6ba..4815cc24cac3be7e9affb0cca99a1284db1ef115 100644 (file)
--- a/folly/test/CacheLocalityBenchmark.cpp
+++ b/folly/test/CacheLocalityBenchmark.cpp
@@ -31,6 +31,7 @@ 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 33af739bd626c916702e881310553b596f11c292..9c326fde4ba320b7f88b198ee8fc5ac8d695da8b 100644 (file)
--- a/folly/test/CacheLocalityTest.cpp
+++ b/folly/test/CacheLocalityTest.cpp
@@ -413,6 +413,7 @@ 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 f7668fc587e5bd68d9270683f3b2e0c8007bafc8..fe59cc02145c47357fb68158a16914119a9cb634 100644 (file)
--- a/folly/test/DeterministicSchedule.cpp
+++ b/folly/test/DeterministicSchedule.cpp
@@ -15,17 +15,15 @@
  */
 
 #include <folly/test/DeterministicSchedule.h>
-
-#include <assert.h>
-
 #include <algorithm>
 #include <list>
 #include <mutex>
 #include <random>
-#include <unordered_map>
 #include <utility>
+#include <unordered_map>
+#include <assert.h>
 
-#include <folly/Random.h>
+DECLARE_ACCESS_SPREADER_TYPE(folly::test::DeterministicAtomic)
 
 namespace folly {
 namespace test {
@@ -141,7 +139,7 @@ int DeterministicSchedule::getRandNumber(int n) {
   if (tls_sched) {
     return tls_sched->scheduler_(n);
   }
-  return Random::rand32() % n;
+  return std::rand() % n;
 }
 
 int DeterministicSchedule::getcpu(unsigned* cpu,

diff --git a/folly/test/SharedMutexTest.cpp b/folly/test/SharedMutexTest.cpp
index 98df9d09cbde1fe0f7da6248d03511fcf6348e3f..8672e665d1ba54412c4c8547e84d37f8824d9b12 100644 (file)
--- a/folly/test/SharedMutexTest.cpp
+++ b/folly/test/SharedMutexTest.cpp
@@ -42,6 +42,11 @@ 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;