[CMake] Closer to compiling with Clang #772
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: This brings us closer to being able to compiling the open source version of Folly with Clang. There are a few other open pull requests that need to be addressed to allow us to compile on Clang without any warnings that aren't already explicitly disabled. See pull requests `745` and `756`.
There was a problem hiding this comment.
@yfeldblum has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
After some thought, perhaps we just need to really address #745 and then this PR isn't needed. The only thing holding that up I believe is removing the internal patched versions of Boost which removed |
KindDragon
added a commit
to KindDragon/folly
that referenced
this pull request
Mar 16, 2018
* Closer to compiling with Clang Summary: This brings us closer to being able to compile the open source version of Folly with Clang. There are a few other open pull requests that need to be addressed to allow us to compile on Clang without any warnings that aren't already explicitly disabled. See pull requests facebook#745 and facebook#756. Closes facebook#772 Reviewed By: simpkins Differential Revision: D7021595 Pulled By: yfeldblum fbshipit-source-id: 350b972c74502ca2bc147f21428c622b19805b89 * cmake: fix checks for compiler warning flags Summary: gcc does not complain about unknown `-Wno-*` flags by default, and older versions of gcc do not support `-Werror=unknown-warning-option`. Therefore these checks for `-Wno-noexcept-type` and `-Wno-nullability-completeness` could incorrectly indicate that the compiler supports these flags. This changes the checks to check against the positive version of the warning flag. Reviewed By: yfeldblum Differential Revision: D7035995 fbshipit-source-id: b1511d31721aeb779859848e20820c8a7fea4faa * subprocess: disable -Wclobbered around spawnInternal() Summary: When compiling with optimization enabled, gcc 7.2 complains about the `childDir` variable potentially being clobbered by the child process after `vfork()`. This shouldn't matter since `childDir` is only used in the child process, and not in the parent after the `vfork()`. Reviewed By: yfeldblum Differential Revision: D7035993 fbshipit-source-id: c80a4e18ad4f629a3d08e310cfd4500ebaf70994 * cmake: fix generation of folly-config.cmake Summary: The install step for folly CMake-based builds installs a folly-config.cmake file that downstream projects can use to depend on folly in their own CMakeLists.txt files. CMakeLists.txt was previously generating the folly-config.cmake file by copying CMake/folly-deps.cmake. This seems wrong, as the installed folly-config.cmake file should not contain `find_package()` statements that depend on CMake modules only available in the folly source. This adds a new folly-config.cmake.in template instead. Reviewed By: yfeldblum Differential Revision: D7035992 fbshipit-source-id: e6023094a807cf481ac49998c6f21b213be6c288 * folly: extend gdb deadlock detector for pthread_rwlock_t Summary: If a pthread_rwlock_t shared mutex is currently write-locked, we can find its owner. However, if it is not write-locked or read-locked (possibly by multiple threads), we will be unable to find the owner. We can extend our deadlock detector to find deadlocks involving write-locked pthread_rwlock_t mutexes as well. Example output: Type "deadlock" to detect deadlocks. (gdb) deadlock Found deadlock! Thread 2 (LWP ...) is waiting on pthread_mutex_t (0x...) held by Thread 3 (LWP ...) Thread 3 (LWP ...) is waiting on pthread_rwlock_t (0x...) held by Thread 2 (LWP ...) Reviewed By: yfeldblum Differential Revision: D6974509 fbshipit-source-id: 8e64cb35f93958a9785070184df4c818b086ff0f * more ConcurrentHashMap deletion tests Summary: Fixes for hazptr deletion on map destruction. Several iteration cases don't properly release their hazard pointers. This isn't disastrous, but means objects may stick around longer than necessary. This diff adds tests and fixes for iteration to ensure we eagerly reset all hazard pointers. Reviewed By: magedm Differential Revision: D6993857 fbshipit-source-id: bee63b3f597d1ed16cae5ed22a971fd4be2e1a77 * Back out "[folly] more ConcurrentHashMap deletion tests" Summary: Original commit changeset: bee63b3f597d. Original revision is breaking integration tests by failing with the SIGSEGV in operator++, please check D6993857 for more details. Reviewed By: anshulverma Differential Revision: D7051998 fbshipit-source-id: e5cfbb22aee5f5ec9871a9a34bb27eb5b0036d1b * Fix folly/experimental/hazptr/ under gcc7 Summary: [Folly] Fix `folly/experimental/hazptr/` under gcc7, which has a bug in emitting TLS init functions for `extern thread_local` variables. Fixes: ``` folly/experimental/hazptr/test/HazptrTest.cpp:642: error: undefined reference to 'TLS init function for folly::hazptr::tls_priv_data_' folly/experimental/hazptr/test/HazptrTest.cpp:642: error: undefined reference to 'TLS init function for folly::hazptr::tls_priv_data_' ``` The trick is to use a template class with `static thread_local` members with member definitions in the same header, and then to use only one instance of that template class. Reviewed By: pixelb, magedm Differential Revision: D7047235 fbshipit-source-id: 0551179a40f13796d6c0bb99aa806c73bd9dd471 * Folly: AsyncSSLSocket good if uninit Summary: This allows to create a Channel with AsyncSSLSocket while evb in not looping yet. Also add ssl channel tests to thrift. Reviewed By: yfeldblum Differential Revision: D6991017 fbshipit-source-id: 0852b18e03a6b9d072cb4ec0182f1a41a5960eef * - comment out unused parameters Reviewed By: everiq, igorsugak Differential Revision: D7046710 fbshipit-source-id: 8e10b1f1e2aecebbfb229c742e214db887e5a461 * Android doesn't have cache data in sysfs Summary: Since sysfs doesn't have cache information for the CPU on Android guard this on Android Reviewed By: yfeldblum Differential Revision: D7051199 fbshipit-source-id: 20fcfa91aa2f642c6cab33b0fb55f121b609d58f * Back out "[codemod] - comment out unused parameters" Reviewed By: igorsugak fbshipit-source-id: 4a93675cc1931089ddd574cacdb15d228b1e5f37 * hazptr: Clear hazard pointers in ~hazptr_array() and ~hazptr_local() Summary: Clear hazard pointers in ~hazptr_array() and ~hazptr_local(). Added tests to detect uncleared hazard pointers when using hazptr_array and hazptr_local. Differential Revision: D6995057 fbshipit-source-id: 5b8fa1d197c3959ed670d0a019959bd57f5df3d0 * Fix master branch broken for Clang Summary: Clang does not support the warning option "-Wclobbered", so we cannot do a pragma push and pop for this warning. So, we push and pop this particular warning for GCC only. As it stands currently, Clang accepts `#pragma GCC` directives. If this sort of thing comes up more in the future, it may make sense for Folly to define a macro for easily disabling warnings for either just GCC or just Clang. simpkins yfeldblum Closes facebook#779 Reviewed By: simpkins Differential Revision: D7059916 Pulled By: yfeldblum fbshipit-source-id: a6d420dc6ae435e3be1229b2bb421910bc8794ff * Add FindLZ4 module Summary: Add a CMake find_package script for LZ4 simpkins Closes facebook#778 Reviewed By: simpkins Differential Revision: D7064988 Pulled By: yfeldblum fbshipit-source-id: 60596ead4b4fd38474dab4c33d4c3fbb0f37549c * Remove SmallLocks dependency from small_vector. Reviewed By: yfeldblum Differential Revision: D7060959 fbshipit-source-id: 914c517b4c8e93c15771f6acefaddce3138776a4 * Fix alignment issues in small_vector. Summary: Packing the structs disregards the alignment of the storage. We should only do this on platforms where we do not need alignment. Reviewed By: yfeldblum Differential Revision: D7058593 fbshipit-source-id: 63b0d86bcf8b4f660ca244b1a4696e3ca64ecd6a * EASY: clang-format fbvector benchmark Summary: format changes, purely generated by clang-format. Differential Revision: D7035579 fbshipit-source-id: 8452e1613adefe7dcfbbed4a3ef97be681790416 * Simplify FsUtilTest.cpp Summary: This test suite used to wrap equality of two paths by using `EXPECT_TRUE` to avoid an old bug in GTest 1.6.0. Now that we use GTest 1.8.0, we can simplify our test. Closes facebook#780 Differential Revision: D7052503 Pulled By: yfeldblum fbshipit-source-id: 5459736fca04cc45acf88a3d7dffc855ad7c9376 * throw_exception to support -fno-exceptions Summary: [Folly] `throw_exception` to support `-fno-exceptions`, switching all invocations of `std::__throw_....`. Removes the need to use the non-portable `bits/functexcept.h`. Reviewed By: mzlee, ot Differential Revision: D7050335 fbshipit-source-id: 53d15639baa268a51f703816f04f314454d64979 * to_signed and to_unsigned Summary: [Folly] `to_signed` and `to_unsigned`, for casting a value to the signed or unsigned version of the value's type. And use them in time conversion functions. Reviewed By: markw65 Differential Revision: D7020215 fbshipit-source-id: 47a6aeeea487cd7d59d23e7ded40cc66dcc57c52 * - Fix unused parameter warning in StlVectorTest.cpp Summary: We are going to enable `-Werror=unused-parameter` flag and I need to manually fix some files so we rest of this process can be automated with a tool called clang-tidy. Reviewed By: yfeldblum Differential Revision: D7071617 fbshipit-source-id: 0165d9c79834b479ad0f598ea64404a86f47dcc6 * Add find module for zstd Summary: Adds a find_package module for Zstd. Closes facebook#785 Reviewed By: pixelb Differential Revision: D7080036 Pulled By: yfeldblum fbshipit-source-id: 018ff087df725de5344a245dea64ce1b813f6938 * doc fix for dynamic::operator[] Summary: folly::dynamic's operator[] was documented as never invalidating iterators. In the object case when a key is implicitly inserted the underlying unordered_map can rehash, which does invalidate the iterators. This diff amends the comment to reflect this case. Reviewed By: yfeldblum Differential Revision: D7085861 fbshipit-source-id: 41ce0497bc60dba9f4529d10010f3b92f9a6c2b3 * Add uint32 type flags Summary: Add DECLARE/DEFINE_uint32 to the portability layer Reviewed By: yfeldblum Differential Revision: D7085534 fbshipit-source-id: 3f6f1bf79371b052706f4b3a03dca1d503b879da * Cut folly/portability/BitsFunctexcept.h Summary: [Folly] Cut `folly/portability/BitsFunctexcept.h`. Reviewed By: simpkins Differential Revision: D7066504 fbshipit-source-id: ed260ab36759a58dee103addd92c6a5a47dac330 * Fix case sensitivity for LZ4 find module Summary: CMake find_package modules are case sensitive. Since the script is called "FindLZ4.cmake" the corresponding call to `find_package` must match LZ4 and not "lz4". For an example of the problem, see the [Travis Log](https://travis-ci.org/facebook/folly/jobs/345661072). Namely: ``` CMake Warning at CMake/folly-deps.cmake:101 (find_package): No "Findlz4.cmake" found in CMAKE_MODULE_PATH. Call Stack (most recent call first): CMakeLists.txt:65 (include) CMake Warning (dev) at CMake/folly-deps.cmake:101 (find_package): Findlz4.cmake must either be part of this project itself, in this case adjust CMAKE_MODULE_PATH so that it points to the correct location inside its source tree. ``` This was not caught on my local machine as I use case-insensitive options in my terminal; in addition, this is not a "hard error" for CMake, so the builds happily continued successfully when 7dc5a4e landed. Closes facebook#782 Reviewed By: simpkins Differential Revision: D7079535 Pulled By: yfeldblum fbshipit-source-id: dd51d8c6ffa804be72c2d9cd264890280dcf5ab9 * Let to_signed be constexpr under C++11 rules Summary: [Folly] Let `to_signed` be `constexpr` under C++11 rules. Reviewed By: simpkins Differential Revision: D7079545 fbshipit-source-id: 69a56f2180f9ec3492cd1ad9cd26dd8bba255442 * more ConcurrentHashMap deletion tests Summary: Re-committing. Original: D6993857 revert: D7051998 Fixes for hazptr deletion on map destruction. Several iteration cases don't properly release their hazard pointers. This isn't disastrous, but means objects may stick around longer than necessary. Adds tests and fixes for iteration to ensure we eagerly reset all hazard pointers. Reviewed By: yfeldblum Differential Revision: D7063727 fbshipit-source-id: 2ae32ead4965b37866096d2a8c6ea8c42c201335 * Fix typos ("and and" -> "and") Reviewed By: yfeldblum Differential Revision: D7091797 fbshipit-source-id: cbba7e5c17e1b6d4446570fb7a2b888db8bcd17e * Tweak throw_exception condition Summary: [Folly] Tweak throw_exception condition, fixing reference to `__GNUC__` and restructuring to make adding checks for alternative compilers more trivial. Reviewed By: mzlee Differential Revision: D7102307 fbshipit-source-id: e4e00ded71f37db8fa93822744b608b42960a58a * Make OpenSSLVersionFinder static methods free functions Summary: getOpenSSLNumericVersion was not static, while getOpenSSLLongVersion() was. Rather than change it to static, these functions should simply be free functions. Reviewed By: yfeldblum Differential Revision: D7094066 fbshipit-source-id: 808a9d169100c9d94d466d9348418a4e8a50c751 * refactor fbvector test and benchmark, enable test, DRY Summary: Previously, there are a number of tests in `FBVectorTestBenchmarks.cpp.h` that are not used anywhere. This diff separates out the tests from the benchmarks and actually include them in `FBVectorTest.cpp`. Reviewed By: yfeldblum Differential Revision: D7103453 fbshipit-source-id: 9427bb1ddcebefb2bbd2c91707fdd572890dc42e * Add find module for Snappy Summary: Add a find_package module for Snappy Closes facebook#783 Reviewed By: simpkins Differential Revision: D7079554 Pulled By: yfeldblum fbshipit-source-id: bf5dc3f3c37367f43fcbf8b40aaa3bb0a030bee5 * Fix DynamicConverter to work with vector<bool> Summary: Introduce a specialization for `std::vector<bool>` as the existing specialization for "other ranges" is not sufficient for binding the proxy iterator (`bit_const_reference`) to an lvalue when iterating over the `vector<bool>`. I do not think you can simply change the type from `const auto&` to `auto&&` when iterating over the container for the "other ranges" specialization. This is because we cannot bind an rvalue to an lvalue reference (which is the function argument type for our `construct` specializations). See facebook#752 and facebook#756 for more info. Closes facebook#781 Reviewed By: andrewjcg Differential Revision: D7078659 Pulled By: yfeldblum fbshipit-source-id: 0089183a3c68d20ed14e483859e950dfb4ae729e * Add an API to AsyncUDPSocket to set DF bit Summary: Some application requires IP not to fragment the the datagram. Add an API to AsyncUDPSocket to set such option. Reviewed By: siyengar Differential Revision: D5421406 fbshipit-source-id: 6cd3ab262f1420e22eb0948e0e2bfef77b7b6e92 * fix multiple invalid-null-argument UBSAN errors in io/Cursor.h Summary: Exposed by UBSAN: {P59121025} {P59121310} {P59121558} Add appropriate checks to avoid the UB. Reviewed By: yfeldblum Differential Revision: D7112776 fbshipit-source-id: 82bd9f85ed4c82aa1b030a38d6ad9358e8f2d38a * Fix unused variable error in AsyncUDPSocket Summary: Some OSes do not define `IP_MTU_DISCOVER` or `IPV6_MTU_DISCOVER`. As such, this would result in the function argument parameter `df` being unused, which is a warning that is then treated as an error. Earlier today (2806dda), this logic was added about setting the DF which broke Mac builds. Closes facebook#791 Reviewed By: igorsugak Differential Revision: D7116863 Pulled By: yfeldblum fbshipit-source-id: 5092891c8977a44ab58723f8cef352dd4eaa1210 * Remove Promise::getFuture from Future-inl.h 1/4 - simple cases Summary: Removing calls to folly::Promise::getFuture from Future-inl.h's simpler cases. Replacing with getSemiFuture. In the case of the collect variants I continue to return a Future, completing inline, until those functions are modified to return SemiFuture. Reviewed By: yfeldblum Differential Revision: D7103938 fbshipit-source-id: 3737887a934cd0fe8f7c1511ae2ba252ea047a49 * Cut build config items for bits/functexcept.h Summary: [Folly] Cut build config items for `bits/functexcept.h`. Folly no longer uses the non-portable functions declared in that internal libstdc++ header. Reviewed By: mzlee Differential Revision: D7119064 fbshipit-source-id: 6879dc6a5bc490726b188d65d7686a2eca136980 * Switch away from using the legacy Synchronized macros Summary: [Folly] Switch away from using the legacy `Synchronized` macros within Folly. Reviewed By: simpkins Differential Revision: D7118324 fbshipit-source-id: 328ecdd572d84bb99a28ddb8689bdc4ae08421a6 * Add compatibility function for X509_STORE_up_ref() Reviewed By: yfeldblum Differential Revision: D7111876 fbshipit-source-id: eba9e29321e0ac89084ceae410bdcb4ebb86f91c * Remove some undefined behaviour from const Lazy. Summary: operator()() const uses const_cast to forward its implementation to operator()(), which invokes UB when writing to the internal state. This change marks the internal state mutable, which means we can delete the const_cast. Reviewed By: yfeldblum Differential Revision: D7089568 fbshipit-source-id: 09f726e6399a95987075d6170ce6fb8103189552 * adjust config for FOLLY_SUPPLY_MISSING_INT128_TRAITS Summary: * remove the redundant `!defined(_WIN32)` condition; * llvm added traits for (u)int128 in https://reviews.llvm.org/D2917; and std::hash in https://reviews.llvm.org/rL266587. Reviewed By: nbronson Differential Revision: D7111138 fbshipit-source-id: 91a05354846575677eddd48c8b8f4e05c53b01cb * add hashers for __int128 and unsigned __int128 Summary: Add hashers for signed and unsigned int128 for platforms that have them. Reviewed By: yfeldblum Differential Revision: D7101875 fbshipit-source-id: ab5df6d06c7f292204318e479d172560ae2a4ec6 * logging: add an FB_LOG_RAW() macro Summary: `FB_LOG_RAW()` is similar to `FB_LOG()` but allows the caller to pass in the log level as a variable, and to specify an alternate file name and line number. This is useful for callers that wish to implement their own logging utility functions that log a message, but pass through their caller's file name and line number. Reviewed By: wez Differential Revision: D7128091 fbshipit-source-id: 1fec158a9312b9c9dc53cf3dc87004d42f9806c6 * CHECK(ev->ev_base) so we get a proper message instead of just a SIGSEGV Summary: CHECK(ev->ev_base) so we get a proper message instead of just a SIGSEGV Reviewed By: yfeldblum Differential Revision: D7118625 fbshipit-source-id: 9426ac54fc32e0d34ad36df97708e3f0ae6043a9 * CMake: folly-deps: gflags is gflags-shared on Fedora facebook#753 Summary: This targets Fedora specifically. More details are available in facebook#753 I personally do not think this is the best way to handle this, but it's something (and works). Still, there might be a chance for this to get merged in? Thank you! /cc yfeldblum Closes facebook#789 Reviewed By: simpkins Differential Revision: D7111531 Pulled By: yfeldblum fbshipit-source-id: 508aeeeb7bfaf2402150cd33f7d6c8b726539053 * Always suspend coroutine before destruction Summary: Suspending the coroutine before it's destroyed prevents a dangling reference to its value. This dangling reference is exposed when running tests that were built with `-O3` with ASAN. To run tests at this level, split the tests up: move tests that cannot be compiled with ASAN at `-O3`, due to https://bugs.llvm.org/show_bug.cgi?id=34289, into their own file. The remaining tests stay where they are, with `-O3` re-enabled. Reviewed By: yfeldblum Differential Revision: D7130814 fbshipit-source-id: bf3e2b8f4a086f1786ab46f453c3dd9171d018e8 * add comment to help people debugging an assertion failure Summary: SharedMutex asserts that all locks have been released before destruction. This diff adds a comment near those asserts making it more clear what they are checking. Reviewed By: yfeldblum Differential Revision: D7137906 fbshipit-source-id: 082a804616950d9a962ee932c9021c453f43a35d * Remove Promise::getFuture from Future-inl.h 2/4 - waitImpl Summary: Replace calls to Promise::getFuture in waitImpl with getSemiFuture. There is a slight behavioural change here in that we now set the executor on the returned future, but this appears to be the correct behaviour. Reviewed By: yfeldblum Differential Revision: D7104627 fbshipit-source-id: fd19c3f6884b560f7d5ea1276f3c80c082dfc53d * Fix bounds check in fbstring::at Summary: From http://en.cppreference.com/w/cpp/string/basic_string/at > Throws std::out_of_range if pos >= size(). Furthermore it was inconsistent between the `const` and non-`const` version. (Note: this ignores all push blocking failures!) Reviewed By: philippv, luciang Differential Revision: D6355911 fbshipit-source-id: 54acbba791fc5a8aeb4fdf46e786f8519fd18701 * fix HAVE_INT128_TRAITS cmake test Summary: C++17 defined integer traits for `__int128` as well as a (trivial) std::hash, this diff adds the check in `FOLLY_SUPPLY_MISSING_INT128_TRAITS`. There was an unintended linebreak in the cmake test which rendered it non functional. This diff fixes it. Original issue: facebook@b2b96f5#comments Reviewed By: yfeldblum Differential Revision: D7138006 fbshipit-source-id: ca1841b9ca847f1f6570b58e95380f50fe092345 * Revert D7130814: [folly] Always suspend coroutine before destruction Summary: This reverts commit bf3e2b8f4a086f1786ab46f453c3dd9171d018e8 bypass-lint An infra SEV is better than not reverting this diff. If you copy this password, see you in SEV Review! cause_a_sev_many_files Differential Revision: D7130814 Original commit changeset: bf3e2b8f4a08 fbshipit-source-id: 8f67c9ec365853f8fbc736e0fd29133fd7653a59 * Remove the spin-lock protecting Future's executor Summary: [Folly] Remove the spin-lock protecting `Future`'s executor. The executor can be set in a few places: * When constructing a new `Promise` and `Future` pair before handing off the `Promise` in `Future::thenImplementation`. * After invoking `SemiFuture::wait` and `Future::wait` in the non-timeout case. * In `SemiFuture` dtor, if it has a core. In the first case, the executor is set in the future thread before being read by the promise thread. In the second case, the executor is set after being read by the promise thread. In the third case, if there is a core, then no callback could have been attached. But there is no race between setting and reading. Reviewed By: djwatson Differential Revision: D7115700 fbshipit-source-id: 5a0f90a9f01a7dcf295f39d323959e91aeae26da * add std::list/deque, small_vector to fbvector benchmark Summary: Adding a few other data structures to the benchmark. Code clean-up: * split `insert` into `insertFront` and `pushBack` * delete current `erase` benchmark, which is mostly spinning since `iters` can easily be larger than the original vector size. Reviewed By: yfeldblum Differential Revision: D7067000 fbshipit-source-id: 3073f7c396888787b49993468c7e6fdac70b77ac * Move folly/MicroSpinLock.h to folly/synchronization/ Reviewed By: igorsugak Differential Revision: D7078903 fbshipit-source-id: 13cdc49369f24cdadc3f9405d1ce609a79ff8988 * An extra deadline check in DynamicBoundedQueue enqueue path Summary: [Folly] An extra deadline check in `DynamicBoundedQueue` enqueue path just for completeness. Reviewed By: magedm Differential Revision: D7151563 fbshipit-source-id: 2d6abd906091db20a650d0f3120bf4b1d8bcf00c * Simplify MemoryIdler pre-idle check Summary: [Folly] Simplify `MemoryIdler` pre-idle check. Reviewed By: magedm Differential Revision: D7151565 fbshipit-source-id: 34ae69f16439640669cd9aaf02a09282a1a1cc3f * Fix waiting in UnboundedQueue Summary: [Folly] Fix waiting in `UnboundedQueue::dequeue` suite. `SaturatingSemaphore::try_wait_until` does not return early with the result `false`, so the loop could result in incorrect long waits. Reviewed By: magedm Differential Revision: D7151562 fbshipit-source-id: cd90583f0b4520464ed1877df8396df43ebbef00 * Fix cUnescape for non-strict cases Summary: Strings terminating in a backslash shouldn't throw if strict is set to false. However, the default implementation results in an undefined behavior, since the iterator goes beyond the end position of the string. Reviewed By: simpkins Differential Revision: D7123155 fbshipit-source-id: 4311037ccdbfd95f402109dcc1b5862e52fb97a1 * Basic coroutine library Summary: This implements Executor-aware coroutine library. Reviewed By: wqfish Differential Revision: D7133189 fbshipit-source-id: 43022e0b4a44378dae670720d8144f2e042f1a54 * fix invalid-null-argument UBSAN error in IOBufQueue.cpp Summary: UndefinedBehaviorSanitizer: invalid-null-argument folly/io/IOBufQueue.cpp:53:38 ``` Add a check that the length is greater than zero. Reviewed By: meyering Differential Revision: D7139027 fbshipit-source-id: 37585092429af52d14c98c193ecc495752bbda1c * Remove Promise::getFuture from Future-inl.h 3/4 - onError Summary: Replaces calls to Promise::getFuture in onError calls. Modifies the returned future to complete on the same executor as the caller. No assumption that the future already has an executor (though it should in principle, the codebase is not ready for that yet). Reviewed By: yfeldblum Differential Revision: D7104850 fbshipit-source-id: ea152102f8ecd612e322b98a54d9440495b14bf8 * Fix waiting in Baton, SaturatingSemaphore Summary: [Folly] Fix waiting in `Baton`, `SaturatingSemaphore`. If condition passes or overall deadline expires, just return. If only a component timeout expires, continue on to the next component. Reviewed By: magedm Differential Revision: D7151564 fbshipit-source-id: 48c33ccc6970604b23221bc94af247945bdc34e5 * Add getSemiFuture and storage of executor to FutureSplitter so that returned future completes on the expected executor. Summary: Remove calls to Promise::getFuture in FutureSplitter test. Add getSemiFuture to FutureSplitter for consistency with Promise. Keep getFuture method but ensure that it completes on the same executor as the input future (or Inline if the executor on the input future was null, due to legacy code). Reviewed By: yfeldblum Differential Revision: D7105806 fbshipit-source-id: 63a919ba9dcf0f189782a3e2a362fad4d86ac91f * Ensure that SemiFuture executor is correctly chained to deal with deferred work. Summary: There was previously a bug in that defer used within a continuation would not correctly chain the deferred executor on the executor in the returned (and hence in the original) folly::Future like it would when .via was called on a SemiFuture. This fixes that situation by returning a semifuture from a continuation by correctly chaining it onto the outer future's executor. Reviewed By: yfeldblum Differential Revision: D7156241 fbshipit-source-id: ed6db3ad0de7a921b72459fcd2b1228114832853 * Remove Promise::getFuture from Future-inl.h 4/4 - window Summary: Modify folly::window to replace Promise::getFuture calls with Promise::getSemiFuture. The requirement for an executor on the semifuture here meant that I had to modify all of the SemiFutures with an executor in the vector. This may come with a small performance hit in some cases, but appears to be behaviourally better. Reviewed By: yfeldblum Differential Revision: D7104920 fbshipit-source-id: 7d105df553c28bb362b37ff64270ee16bf59fbd6 * Fix a GuardPageAllocator range check Summary: The range check to identify its "own" memory could inadvertently pick up an allocation from outside the range, resulting in an alignment assertion later on, and causing us to permanently leak the memory (when assertions are disabled). Reviewed By: alikhtarov, andriigrynenko Differential Revision: D7146394 fbshipit-source-id: d9d8b63eea1ffa045e1bd07c0258d1f2b089e9bf * toUnsafeFuture to avoid propagating InlineExecutor through caller code during transition. Summary: toUnsafeFuture is intended to return a future that is the same as .via(&folly::InlineExecutor::instance()) without propagating InlineExecutor throughout future-using code during a transition to SemiFuture. This will highlight call sites better and encourage either fixing, or making clear that this non-deterministic behaviour is the intent. Reviewed By: yfeldblum Differential Revision: D7162989 fbshipit-source-id: c7092a53560e05d463d9170be254a50d23cc6ef7 * free up existing session in setSSLSession Summary: Free up the session if its not null before assigning a new one Reviewed By: shamdor-fb Differential Revision: D7169809 fbshipit-source-id: fd026e8688525a764b9937c7f4b7bfb0cdece9eb * cmake: define interface include directories Summary: Define interface include directories for the folly target. This lets downstream dependencies of folly list Folly::folly in target_link_libraries() and have the correct include directories be added automatically. Reviewed By: yfeldblum Differential Revision: D7162335 fbshipit-source-id: 2a6aad57142e038450fd35a94446f5b865029155 * define FOLLY_ATTR_WEAK in CPortability.h Summary: Define a FOLLY_ATTR_WEAK helper macro in CPortability.h, and update existing code in folly that declares functions as weak to use this new macro. This will make it easier to declare weak functions in other files in the future. Reviewed By: yfeldblum Differential Revision: D7164834 fbshipit-source-id: 87e03401400f2912fcd5e719af0d667251cd642b * logging: move test utility functions into a helper module Summary: Move some utility functions from ConfigParserTest and ConfigUpdateTest into a new ConfigHelpers module. This eliminates some duplicated logic and will make it easier to re-use these functions in other tests in the future. Reviewed By: yfeldblum Differential Revision: D7164831 fbshipit-source-id: 115a227e4cb2db486f57079483b8ae9aadaa0187 * Let all Promise<T> be friend of Task Summary: [Folly][coro] Let all `Promise<T>` be friend of `Task`. The code would not compile: ```lang=c++ coro::Task<void> taskVoid() { co_await task42(); co_return; } ``` ``` folly/experimental/coro/Promise.h:84:28: error: 'viaInline' is a private member of 'folly::coro::Task<int>' return std::move(task).viaInline(executor_); ^ folly/experimental/coro/tests/CoroTest.cpp:40:3: note: in instantiation of function template specialization 'folly::coro::Promise<void>::await_transform<int>' requested here co_await task42(); ^ folly/experimental/coro/Task.h:60:13: note: declared private here Future<T> viaInline(folly::Executor* executor) && { ^ ``` Reviewed By: andriigrynenko Differential Revision: D7178238 fbshipit-source-id: 7dca6834ac56f4c9bdb4d702996b51e932f2aae6 * Suppress deprecation warning in Promise.h Summary: [Folly][coro] Suppress deprecation warning in `Promise.h`. `Try(std::exception_ptr ep)` is deprecated. The underlying behavior doesn't improve much with this diff. But there would be a whole lot of warnings once `Promise.h` gets included by more files. Reviewed By: andriigrynenko Differential Revision: D7178381 fbshipit-source-id: 9f45706283a5aaa436cbab3e7b02e085bdc8c792 * logging: add a LoggerDB::getFullConfig() method Summary: This method can be used to get configuration about all currently defined log categories, including ones that are using the default config settings. The existing LoggerDB::getConfig() method only returns information about categories that have non-default config settings. Reviewed By: yfeldblum Differential Revision: D7164833 fbshipit-source-id: e1c043553dc4f69be58c33f2bc5b1a63763f0984 * logging: fix the open source build Summary: Add ConfigHelpers.cpp to the folly_test_support library. This was accidentally broken in D7164831, but wasn't caught due to infrastructure errors kicking off the continuous OSS builds. Reviewed By: yfeldblum Differential Revision: D7186076 fbshipit-source-id: f13bdd920bb289ae19fdb46e265123fe088b42c3 * Support custom comparator in EvictingCacheMap Summary: Allow passing in a custom comparator to compare keys Reviewed By: yfeldblum, aary Differential Revision: D7175777 fbshipit-source-id: e1e8d836a908b595a74b172b04ca847a5c5eb435 * Add a buckets + size seqlock Summary: There is currently an unnecessary data dependency loading the current backing buckets + size, reading the size, then again indexing in to the array. Rework the Buckets struct to make a single allocation, and store the bucket_count_ in the Segment directly. The bucket pointer and segment are protected by a seqlock. Reviewed By: ot Differential Revision: D7167919 fbshipit-source-id: 10ddceffad19f54c790b3ab7a87260445571f81d * folly::hasher<T> should convert args to T before hashing Summary: folly::hasher<T> for integral and floating point types T uses a different algorithm depending on the key type. At the moment this is taken from the actual argument type, rather than the explicitly provided one. This is likely to be confusing. Reviewed By: yfeldblum Differential Revision: D7194941 fbshipit-source-id: efa3e4bbad62e53842ec6fb80d316706324e98aa * msvc2015u3: extend D4988025 fix to one more place Summary: This just extends D4988025 to one more location, which wasn't covered in the initial fix. Reviewed By: yfeldblum Differential Revision: D7181160 fbshipit-source-id: 8dd0866d916674870f04252a80725ecce0cd69e1 * Make via a free function Reviewed By: yfeldblum Differential Revision: D7203193 fbshipit-source-id: 440daa2f79a919aa95ae3fdb4006f8d501c0ad93 * logging: add a getBaseLoggingConfig() function Summary: Update folly::initLogging() to call getBaseLoggingConfig() to get a base configuration string that is applied before the argument that it was called with. getBaseLoggingConfig() is defined as a weak symbol, and is intended to be overridden on a per-executable basis if desired. This allows individual programs to control their default logging settings, but still let them be overridden via a command line flag. The command line flag is used to update this base configuration. The command line flag can fully override all of the base configuration settings if desired, but settings from the base config that are not overridden will still be used. For example, if the base configuration defines settings for the categories "foo" and "bar", and the command line flag defines settings for the categories "bar" and "wub", the base configuration settings for "foo" will still be used. Reviewed By: yfeldblum Differential Revision: D7164832 fbshipit-source-id: e172f746d7bd004948872adbbb87c597765e283c * clang-format in preparation for other changes Summary: This diff clang-formats a couple of files in preparation for additional changes. Reviewed By: yfeldblum, shixiao Differential Revision: D7197983 fbshipit-source-id: 8995bdaca29bcc44cd5379fc64c76cee89635ac1 * add IsAvalanchingHasher trait Summary: This diff adds folly::IsAvalanchingHasher<H, K>, which identifies hash functors that exceed the standard's quality requirement by also being avalanching. This is useful for code that wants to map hash values onto a restricted range or compute a secondary hash value without doing extra work. Reviewed By: yfeldblum Differential Revision: D7180217 fbshipit-source-id: 8c402937d0a654c0ec32c62666e9dc4e0943f769 * Make hasher<float> avalanching Summary: It currently only sets the lower 32 bits. Reviewed By: nbronson, philippv Differential Revision: D7216798 fbshipit-source-id: 090f6dba990eeb7f1a9cc56e8615161c11eea026 * SequencedExecutor Reviewed By: phoad Differential Revision: D7220254 fbshipit-source-id: 3a2a49fd17d0aca4acc590c1c16db75ace472216 * Temporarily remove onError returning a future that completes on the original executor due to production issues. Summary: https://fb.facebook.com/groups/fbcode.investigations/permalink/407299856363159/ showed issues coming from this diff. The change to return an executor-tied future from onError was intentional, and correct behaviour, but it was a change that seems to have caused issues. Temporarily removing it to rethink how we should approach it. Reviewed By: andriigrynenko Differential Revision: D7219231 fbshipit-source-id: 95b91f3a5ea7d854d5df052e6cd5851c7e6fbdab * Cut dead iterator-traits ifdefs in fbstring Summary: [Folly] Cut dead iterator-traits `ifdef`s in `fbstring`. Reviewed By: elsteveogrande Differential Revision: D7195605 fbshipit-source-id: 7a398cffab153b1571064a7d9e610e844f33bc20 * Rewrite allocators for the era of std::allocator_traits Summary: [Folly] Rewrite allocators for the era of `std::allocator_traits`. Provide minimal interfaces which comply with all required elements of C++ concept `Allocator`. Change all (*) uses to use `std::allocator_traits` consistently, as is required of all allocator-aware library types. * Rename `SysAlloc`. * Replace `StlAllocator` with `CxxAllocatorAdaptor` with stricter semantics. * `Arena` is no longer a C++ Allocator because it is not freely copyable. Change code which used it as a C++ Allocator to use `CxxAllocatorAdaptor` instead. * `ThreadCachedArena` likewise. (*) Hopefully. Reviewed By: nbronson Differential Revision: D7208794 fbshipit-source-id: 270588c9c3d817f4abd9fb49eed5eb9f03f96da2 * Fix propagate_const under gcc49 Summary: [Folly] Fix `propagate_const` under gcc49. Reviewed By: andrewjcg Differential Revision: D7230851 fbshipit-source-id: 0cd4df15d5a0f7ae06e26a82ca725b30d479f458 * NotificationQueue benchmark Summary: NotificationQueue benchmark Reviewed By: yfeldblum, djwatson Differential Revision: D7163944 fbshipit-source-id: f7fe2124b8c61ff3ab39be94d9c50c82a6f8816b * Add folly::Optional<T> UnboundedQueue::try_dequeue() method Summary: Add folly::Optional<T> UnboundedQueue::try_dequeue() method Reviewed By: magedm Differential Revision: D7164049 fbshipit-source-id: 659de2d26067bb1a5bdea50f4cd7711e6955ba61 * Use unbounded queue in NotificationQueue Summary: Use unbounded queue in NotificationQueue Reviewed By: djwatson Differential Revision: D7164130 fbshipit-source-id: d1b445d6730cbb2a68ad78506abe0165e28f5811 * Add timed_wait Summary: Add try_wait_for and try_wait_until. Algorithm - We could use a doubly-linked list, but this would double the number of contended CAS for push/post. Instead, assume timeouts are infrequent, and just walk the list from the head to remove nodes. A lock bit is added to the head, which is taken only on timeout. We assume timeouts are infrequent. If a concurrent post() removes the node before the timeout can remove it, we must wait for the corresponding post() (but it is likely to arrive soon), instead of timing out. Reviewed By: magedm Differential Revision: D7167894 fbshipit-source-id: ea5242098d9ccd286a72fade6292223e95c44a81 * SSLContext: Report error codes in hexadecimal. Summary: For compatiblity with `openssl errstr`. Reviewed By: yfeldblum Differential Revision: D7246053 fbshipit-source-id: dcf77b67ae3b3dd7b6b85e9b73f2d382e59aa7b9 * Suppress -Wshadow-compatible-local in folly/test/stl_tests/StlVectorTest.cpp Summary: [Folly] Suppress `-Wshadow-compatible-local` in `folly/test/stl_tests/StlVectorTest.cpp`. That test source file is heavily macro-driven, and using `SCOPED_TRACE` repeatedly in macros can lead to violations of `-Wshadow-compatible-local`. Reviewed By: meyering Differential Revision: D7231029 fbshipit-source-id: 122e1d86032cf884a83ef1716fa48b8f4864a363 * Revert D7164130 Reviewed By: djwatson Differential Revision: D7251343 fbshipit-source-id: e5ce0c897f0d96c8717d544ac98a1faf12f08c02 * Missing SequencedExecutor.h Summary: When compiling using autoconf, automake, make and make install, SequencedExecutor.h is not installed automatically. Closes facebook#799 Reviewed By: meyering Differential Revision: D7241820 Pulled By: yfeldblum fbshipit-source-id: 03c608c8ece2e7870a7bacce399ecc9455691948 * Separate Try-taking defer and value-taking deferValue in SemiFuture. Summary: This avoids ambiguity with an auto-parameterised lambda and leads on to a consistent use of deferError in a followup diff. Reviewed By: yfeldblum Differential Revision: D7224490 fbshipit-source-id: df410cd97ca0991db8fcb46267dd6a4236f3f6a3 * F14 hash table in folly Summary: F14 is a 14-way probing hash table that resolves collisions by double hashing. Up to 14 keys are stored in a chunk at a single hash table position. SSE2 vector instructions are used to filter within a chunk; intra-chunk search takes only a handful of instructions. "F14" refers to the fact that the algorithm "F"ilters up to "14" keys at a time. This strategy allows the hash table to be operated at a high maximum load factor (12/14) while still keeping probe chains very short. F14 provides compelling replacements for most of the hash tables we use in production at Facebook. Switching to it can improve memory efficiency and performance at the same time. The hash table implementations widely deployed in C++ at Facebook exist along a spectrum of space/time tradeoffs. The fastest is the least memory efficient, and the most memory efficient is much slower than the rest. F14 moves the curve, simultaneously improving memory efficiency and performance when compared to the existing algorithms, especially for complex keys and large maps. Reviewed By: yfeldblum Differential Revision: D7154343 fbshipit-source-id: 42ebd11b353285855c0fed5dd4b3af4620d39e98 * Add deferError to SemiFuture. Summary: deferError adds an error handling callback to SemiFuture, enqueued on the internal deferred executor, and thus will run on the chained executor or inline with .get() as for defer. Reviewed By: yfeldblum Differential Revision: D7243193 fbshipit-source-id: 9fe7dab9fbc8b236d717b058c81a303d87fa9253 * Make EventBase subclass IOExecutor (as in docstring) Summary: EventBase meets the definition of IOExecutor (it is an executor that "manages" at least one EventBase) and is already claimed to be one in the docstring. Reviewed By: yfeldblum Differential Revision: D7064481 fbshipit-source-id: 026e0e4c24d6fb15a32bc4772b35689f830ed944 * Allowing folly::dynamics of object/array type to be hashed Summary: Currently, an error is thrown if a user tries to hash an instance of `folly::dynamic` which is of type `object` or `array`. Primitive underlying types are fine. There doesn't seem to be a strict reason for this, as objects and arrays are just as hashable as primitive types. Additionally, the inability to hash here causes some unnecessary drawbacks (e.g. interacting with `unordered_map`s). This diff uses the existing dependency on `folly/hash/Hash.h` to start allowing `folly::dynamic` objects/arrays to be hashed as well. Reviewed By: ot Differential Revision: D7219760 fbshipit-source-id: 1dff3f99bdacb66f719c614040cbd668ce51a33e * fix F14 MSVC build Summary: F14 was missing the MSVC portability shims, and also needs to use an MSVC-specific intrinsic to get the high word of a 64-bit multiply. Reviewed By: yfeldblum Differential Revision: D7263349 fbshipit-source-id: f47ceb2b2370d9bef1409e4790b236752c9f6ac4 * Enable -Wunused-value Summary: [Folly] Enable `-Wunused-value`. Reviewed By: Orvid Differential Revision: D7266246 fbshipit-source-id: dc9b85a5f8ce67802cc2fa94746a26529012ec22 * Added forward declaration of ObserverCreator Summary: For some reason, compilation of certain Observable code fails in opt mode with the error that Observer::core_ is private. It seems that ObserverCreator is not recognized as a friend of Observer. This forward declaration of ObserverCreator seems to fix this issue. Reviewed By: yfeldblum Differential Revision: D7268318 fbshipit-source-id: 1ea76efb0f50cb8a6b28147305ec7547f55338b8 * Parameterize AlignedSysAllocator by a policy Summary: [Folly] Parameterize `AlignedSysAllocator` by a policy. Reviewed By: ot Differential Revision: D7246946 fbshipit-source-id: 5b6a89707c242e49bdbafba9dec6df4c65ef385c * lift Default{Hasher, KeyEqual, Alloc} out of detail namespace Summary: Expose the default structs in `folly::f14` namespace so that users _could_ potentially refer to them. Reviewed By: nbronson Differential Revision: D7271846 fbshipit-source-id: 81a193209e59ed80260098ed97c99a7b6f2ad151 * Fix fork behavior Summary: hazptr_priv_list needs to be reinitialized on fork, as well as ODR violations. This broke stuff when trying to land D7164130 TL;DR the thread_locals need to be in a .cpp file, the templates don't always get merged correctly, especially in the face of dlopen. You can, however, use a thread_local * pointer cache to the object. This is basically what folly::SingletonThreadLocal does. Folly's ThreadLocal runs destructors when forking even too, so nothing special is required. Reviewed By: yfeldblum Differential Revision: D7256905 fbshipit-source-id: c817167b5c3db27fa929feaa39295fd939c1cb4c * eachTryTo, takeWhile Reviewed By: yfeldblum Differential Revision: D7257019 fbshipit-source-id: 2fe54fe1d42bfe81b2f6f8e43ae0f7e738e93edd * add __builtin_ctz and __builtin_ctzl Summary: Add missing WIN32 compat builtins Reviewed By: Orvid Differential Revision: D7276240 fbshipit-source-id: 094c0861126b3477e9112987f033edacbde4aca4 * expose Default{Hasher, KeyEqual, Alloc} to platforms without intrinsics available Summary: Expose Default{Hasher, KeyEqual, Alloc} to platforms without intrinsics available Reviewed By: yfeldblum Differential Revision: D7277896 fbshipit-source-id: 92f46aef9ab0926c32c85e29e368f7c0b58c2b3e * Make EventBase extend ScheduledExecutor Summary: This makes it easier to use EventBase in cases where classes require ScheduledExecutor instead of Executor. Previously the client would have to implement a wrapper around EventBase. Reviewed By: yfeldblum Differential Revision: D7179869 fbshipit-source-id: 21730a56ed5b71fd731ffd272f9f3752b70d63ce * Add constexpr_add/sub_overflow_clamped in folly/ConstexprMath.h Summary: Provide UBSAN-clean add/sub function on integral types. Helps to avoid overflow. Reviewed By: yfeldblum Differential Revision: D7230723 fbshipit-source-id: 10fa54b405f9643dfbbb1940cf100516fd1341e1 * Use EXPEDITED by default Summary: MEMBARRIER_CMD_SHARED is too slow, use EXPEDITED (or the mprotect hack if kernel is too old). hazptr has been using EXPEDITED for a while, the only other users are RCU and read_mostly_shared_ptr Reviewed By: yfeldblum, osandov Differential Revision: D7280190 fbshipit-source-id: 2a9aeab035f8996566b557d7f406686c361a37d7 * Fix SingletonThreadLocal use from headers Summary: [Folly] Fix `SingletonThreadLocal` use from headers included by separate dynamically-loaded modules. There is no need in `SingletonThreadLocal` specifically to check for double-registration and `SingletonThreadLocal` does not support mocks, so `LeakySingleton` may be bypassed entirely. Reviewed By: djwatson Differential Revision: D7283390 fbshipit-source-id: 45295c4dab233b9373ed32a47d4c80832b0f9eff * Alias std::launder on MSVC Summary: [Folly] Alias `std::launder` on `MSVC` to avoid ambiguity. Fixes facebook#802. Reviewed By: Orvid Differential Revision: D7286052 fbshipit-source-id: 7cd89c9ad236a433f53fbdb9e2e024b85105827e * switch from assert to FOLLY_SAFE_DCHECK Summary: assert(x) doesn't mark x as used in prod builds, causing problems for builds that warn on unused variables and promote warnings to errors. This diff changes assert calls to FOLLY_SAFE_DCHECK, which is better in this regard. Reviewed By: phoad, shixiao Differential Revision: D7291084 fbshipit-source-id: 1ae48a271f30182ac8a1c2cf126af0e6b4fd0a25 * Fix the build on Windows due to MTU_DISCOVER Summary: Windows defines `IP_MTU_DISCOVER`, but does it differently from everywhere else, so disable it unless everything required is defined. Reviewed By: yfeldblum Differential Revision: D7298640 fbshipit-source-id: 8ecc2dfd9ba83f31e9d72e0f42fb4fbf8f3e50b5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This brings us closer to being able to compile the open
source version of Folly with Clang. There are a few other
open pull requests that need to be addressed to allow us to
compile on Clang without any warnings that aren't already
explicitly disabled. See pull requests #745 and #756.
@yfeldblum Sorry for the duplicate of #768 -- I had rebased and force-pushed the reverting of the original changeset to then apply the new changeset as requested from the internal code review feedback. The result was that the PR got closed due to showing zero commits on the branch and then it wouldn't reopen despite force pushing to the branch with this changeset. Apologize for that.