From 6c76acfc86177dd2570bf80411baa47dd2b74c72 Mon Sep 17 00:00:00 2001 From: Eric Niebler Date: Sat, 25 Jun 2016 11:48:12 -0700 Subject: [PATCH] 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: D3447679 fbshipit-source-id: 22c90c19e2c7a9b6d772058f2c7e350b890b6c0a --- folly/Fingerprint.h | 25 +++++++++++++-- folly/SharedMutex.cpp | 11 ++++--- 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, 88 insertions(+), 57 deletions(-) diff --git a/folly/Fingerprint.h b/folly/Fingerprint.h index f1980ae2..a4400d5d 100644 --- a/folly/Fingerprint.h +++ b/folly/Fingerprint.h @@ -51,12 +51,31 @@ namespace folly { namespace detail { + template 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 +const uint64_t FingerprintTable::poly[1 + (BITS - 1) / 64] = {}; +template +const uint64_t FingerprintTable::table[8][256][1 + (BITS - 1) / 64] = {}; + +#define FOLLY_DECLARE_FINGERPRINT_TABLES(BITS) \ + template <> \ + const uint64_t FingerprintTable::poly[1 + (BITS - 1) / 64]; \ + template <> \ + const uint64_t FingerprintTable::table[8][256][1 + (BITS - 1) / 64] + +FOLLY_DECLARE_FINGERPRINT_TABLES(64); +FOLLY_DECLARE_FINGERPRINT_TABLES(96); +FOLLY_DECLARE_FINGERPRINT_TABLES(128); + +#undef FOLLY_DECLARE_FINGERPRINT_TABLES + +} // namespace detail /** * Compute the Rabin fingerprint. diff --git a/folly/SharedMutex.cpp b/folly/SharedMutex.cpp index e7c5267e..efabaa4e 100644 --- a/folly/SharedMutex.cpp +++ b/folly/SharedMutex.cpp @@ -14,9 +14,10 @@ * limitations under the License. */ -#include "SharedMutex.h" +#include -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; +template class SharedMutexImpl; +} diff --git a/folly/SharedMutex.h b/folly/SharedMutex.h index 15dc91e6..2d71a884 100644 --- 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 class Atom, + bool BlockImmediately> +typename SharedMutexImpl:: + DeferredReaderSlot + SharedMutexImpl:: + deferredReaders[kMaxDeferredReaders * kDeferredSeparationFactor] = + {}; + +template < + bool ReaderPriority, + typename Tag_, + template class Atom, + bool BlockImmediately> +FOLLY_TLS uint32_t + SharedMutexImpl:: + tls_lastTokenlessSlot = 0; typedef SharedMutexImpl SharedMutexReadPriority; typedef SharedMutexImpl 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; +extern template class SharedMutexImpl; + } // namespace folly diff --git a/folly/detail/CacheLocality.cpp b/folly/detail/CacheLocality.cpp index e75120d6..c62e0ac8 100644 --- a/folly/detail/CacheLocality.cpp +++ b/folly/detail/CacheLocality.cpp @@ -28,8 +28,6 @@ #include #include -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 SequentialThreadId::prevId(0); - -template <> -FOLLY_TLS size_t SequentialThreadId::currentId(0); +template struct SequentialThreadId; #endif /////////////// AccessSpreader - -template <> -Getcpu::Func AccessSpreader::pickGetcpuFunc() { - auto best = Getcpu::resolveVdsoFunc(); - return best ? best : &FallbackGetcpuType::getcpu; -} +template struct AccessSpreader; } // namespace detail } // namespace folly diff --git a/folly/detail/CacheLocality.h b/folly/detail/CacheLocality.h index 1ede43cb..c9baf528 100644 --- a/folly/detail/CacheLocality.h +++ b/folly/detail/CacheLocality.h @@ -161,6 +161,16 @@ struct SequentialThreadId { static FOLLY_TLS size_t currentId; }; + +template