From 0f2aacae3d558f5cf54751b170c4e50d5483cc29 Mon Sep 17 00:00:00 2001
From: Jinlong Zhou <jlz@fb.com>
Date: Thu, 16 Jun 2016 13:26:54 -0700
Subject: [PATCH] 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
---
 folly/Fingerprint.h                   | 14 ++-------
 folly/SharedMutex.cpp                 |  9 +++---
 folly/SharedMutex.h                   | 31 +++++-------------
 folly/detail/CacheLocality.cpp        | 16 ++++++++--
 folly/detail/CacheLocality.h          | 45 ++++++++++-----------------
 folly/test/CacheLocalityBenchmark.cpp |  1 +
 folly/test/CacheLocalityTest.cpp      |  1 +
 folly/test/DeterministicSchedule.cpp  | 10 +++---
 folly/test/SharedMutexTest.cpp        |  5 +++
 9 files changed, 56 insertions(+), 76 deletions(-)

diff --git a/folly/Fingerprint.h b/folly/Fingerprint.h
index c7a20fb7..f1980ae2 100644
--- 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 23266b7c..e7c5267e 100644
--- 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 2d71a884..15dc91e6 100644
--- 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 c62e0ac8..e75120d6 100644
--- 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 c9baf528..1ede43cb 100644
--- 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 81f094d9..4815cc24 100644
--- 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 33af739b..9c326fde 100644
--- 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 f7668fc5..fe59cc02 100644
--- 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 98df9d09..8672e665 100644
--- 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;
-- 
2.34.1