1. 08 Mar, 2021 1 commit
    • Aaryaman Sagar's avatar
      Explicitly qualify atomic_wait and atomic_notify function calls · 8477e256
      Aaryaman Sagar authored
      Summary:
      `std::atomic_wait()` and `std::atomic_notify_one()` are marked as unavailable.
      Not really sure what that means, but it seems to be breaking some open source
      builds https://github.com/facebook/folly/issues/1527.  Explicitly qualify the
      calls into those functions to try and fix the build break.
      
      Since we cannot conditionally import either of the above (because they are
      marked as unavailable), we can't rely on the standard implementations.  To
      prevent ADL from kicking in when the standard-library defines these, we fully
      qualify these and use `tag_invoke` for the customization points used in tests.
      
      Reviewed By: davidtgoldblatt
      
      Differential Revision: D26742072
      
      fbshipit-source-id: 9f44bbfd37530f5ecffa3e03d673cfb1df953299
      8477e256
  2. 07 Mar, 2021 2 commits
    • Yedidya Feldblum's avatar
      to_ascii, to_ascii_size · 89a3d4a7
      Yedidya Feldblum authored
      Summary:
      Generic ascii-ification functions `to_ascii` and `to_ascii_size` for varying unsigned integer types over varying integer bases.
      
      Has minimal dependencies. Is explicitly async-signal-safe.
      
      Reviewed By: kennyyu
      
      Differential Revision: D26593764
      
      fbshipit-source-id: b1432bee8ca8eb0a5304943c4e231e201425f14a
      89a3d4a7
    • Yedidya Feldblum's avatar
      Executor::invokeCatchingExns · 2f6126e6
      Yedidya Feldblum authored
      Summary: Executors just carry on after a task they are running fails, although they try to log something marginally useful. Deduplicate the logic.
      
      Reviewed By: aary, ot, philippv
      
      Differential Revision: D26745058
      
      fbshipit-source-id: 90fa558c89591b8a2d08e6820388970dee8811be
      2f6126e6
  3. 06 Mar, 2021 3 commits
    • Alfred Fuller's avatar
      Update include categories in clang-format canary · 688c8aaa
      Alfred Fuller authored
      Summary:
      This just fixes the include order within groups of includes, so is significantly less aggressive than the other codemod which regroups includes.
      
      I've manually ensured that:
      - groups are split when trivial.
      - glog is not reordered relative to other folly headers (seems to cause down stream issues)
      - portability headers are not reordered (seems to cause down stream issues)
      other diffs will finish the regrouping and ordering of those groups.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26405757
      
      fbshipit-source-id: 9b79a91a41a4e6ac677ed0cb7fb24dfb7c8093d2
      688c8aaa
    • Andrii Grynenko's avatar
      InlineLikeExecutor · dff3d40c
      Andrii Grynenko authored
      Summary: Tag all inline-like executors, so that there's a single way to detect them in places where we don't want inline executors.
      
      Differential Revision: D26564118
      
      fbshipit-source-id: 1a55c03bee0a0a56328175c016d3880a86eab8b1
      dff3d40c
    • Yedidya Feldblum's avatar
      no need to test for __has_include · 0bdb2b67
      Yedidya Feldblum authored
      Summary: Folly may assume all compilers have `__has_include`. Remove the single case of testing for it before using it.
      
      Reviewed By: simpkins
      
      Differential Revision: D26788728
      
      fbshipit-source-id: a11b47f6730700d81141d14c627e9d414ee6f68d
      0bdb2b67
  4. 05 Mar, 2021 10 commits
    • Alan Frindell's avatar
      Add AsyncSocket::UniquePtr ctor coro::Socket · b1ab506c
      Alan Frindell authored
      Summary: This diff is part of a series cleaning up coro::Socket
      
      Reviewed By: yairgott
      
      Differential Revision: D26837067
      
      fbshipit-source-id: e7ef587a6f859a167404b3eb2c8399544aa1d092
      b1ab506c
    • Ruslan Sayfutdinov's avatar
      Define S_IRXO, S_ISREG in SysStat.h · 696d65f7
      Ruslan Sayfutdinov authored
      Reviewed By: yfeldblum
      
      Differential Revision: D26848785
      
      fbshipit-source-id: 441aeac55f51ee8ec5e2316288f2c1f778a60dda
      696d65f7
    • Shai Szulanski's avatar
      folly::coro::makeUnorderedAsyncGeneratorFromAwaitable[Try]Range · 33b99b77
      Shai Szulanski authored
      Summary:
      Allows waiting for a range of SemiAwaitables while receiving progress incrementally.
      Often requested as a repeatedly-invokable collectAll.
      
      Uses an external AsyncScope to avoid the following sharp edge with using an internal scope (demonstrated in V1 of this diff): if the AsyncGenerator is destroyed without being fully drained we'd have to block on the scope, which can deadlock.
      
      Reviewed By: helfman
      
      Differential Revision: D25924855
      
      fbshipit-source-id: 46dd7c64d58560f57a7c18e35c159beb984286a7
      33b99b77
    • Yedidya Feldblum's avatar
      mod all folly::try_and_catch<std::exception> · e5ea61d1
      Yedidya Feldblum authored
      Summary: Mod all such cases not to pass any exception type list since such cases presumably want to catch all exceptions.
      
      Reviewed By: ericniebler
      
      Differential Revision: D26712856
      
      fbshipit-source-id: 1b5de869da5701a6bec85770f11c88195d7e62d8
      e5ea61d1
    • Henry Wang's avatar
      Use #if FOLLY_HAVE_SO_TIMESTAMPING · 70d84dd8
      Henry Wang authored
      Summary:
      D24094832 (https://github.com/facebook/folly/commit/842ecea531e8d6a90559f213be3793f7cd36781b) enabled code blocks which were currently not supported with SGX build mode, resulting in fbenclave build errs [T85891699].
      
      Our SGX targets depend on folly and are built with a custom toolchain. Because of our dep constraints we don't support all features. We typically try to toggle certain folly features, for example: https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/tools/buckconfigs/fbcode/sgx.bcfg?commit=8981fc7187d8b2484a764e9277a1efa082a13f50&lines=36
      
      We were unable to disable these blocks of code because this macro was defined to be 1. We would typically just add the compiler flag: -DFOLLY_HAVE_SO_TIMESTAMPING=0. But doing causes redefinition.
      
      Reviewed By: bschlinker
      
      Differential Revision: D26765404
      
      fbshipit-source-id: 06d800efa7f23d9235a019ed7cc8fe4cde53c91c
      70d84dd8
    • Giuseppe Ottaviano's avatar
      Ensure that Function is noexcept-destructible · 694a416e
      Giuseppe Ottaviano authored
      Reviewed By: yfeldblum, philippv, luciang
      
      Differential Revision: D26832361
      
      fbshipit-source-id: 9f67a0af8c3c939cbfda1bd46806849e164e5354
      694a416e
    • Yedidya Feldblum's avatar
      try_and_catch with no type list · 94465a38
      Yedidya Feldblum authored
      Summary: Since most uses just specify `std::exception` and really just want to catch everything.
      
      Reviewed By: ericniebler
      
      Differential Revision: D26712827
      
      fbshipit-source-id: b33c1a311a839a50d81b9bc6c2b441908130dec6
      94465a38
    • Andrii Grynenko's avatar
      Don't discard tasks from executor when draining FiberManager · 2b8facde
      Andrii Grynenko authored
      Summary: FiberManager depends on those tasks to be run to know when it's safe to shut down (i.e. all outstanding tasks should be run).
      
      Differential Revision: D26811875
      
      fbshipit-source-id: 98b59ad28e25e7e47197e23ad3209349fdbfbb44
      2b8facde
    • Christopher Wei's avatar
      Add indent size for json pretty printing · 579e7cb2
      Christopher Wei authored
      Summary: Adds `pretty_formating_indent_size` option which is used to change the indentation size when used with `pretty_formatting`
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26793086
      
      fbshipit-source-id: 53efea33d7daa5c8f8840fc7f087d2d976ed3eda
      579e7cb2
    • Yedidya Feldblum's avatar
      revise demangle · 2690cff7
      Yedidya Feldblum authored
      Summary:
      Revise `folly::demangle` and its test suite.
      
      As a few items:
      * Minimize preprocessor conditional compilation.
      * Make the tests work for all combinations.
      
      Differential Revision: D26551208
      
      fbshipit-source-id: 422f6f96632c4dfc3f2e51f7b3ae8855cdb3ab30
      2690cff7
  5. 04 Mar, 2021 8 commits
    • Ruslan Sayfutdinov's avatar
      define NAME_MAX for Windows · 3fdabf98
      Ruslan Sayfutdinov authored
      Summary:
      If we have `PATH_MAX` we can probably have `NAME_MAX` as well.
      
      It should be exact equivalent of `NAME_MAX` on Windows:
      https://docs.microsoft.com/en-us/cpp/c-runtime-library/path-field-limits?view=msvc-160
      
      Reviewed By: Orvid
      
      Differential Revision: D26784071
      
      fbshipit-source-id: 1e28d5de4358efded182db85f0382939c96a4dce
      3fdabf98
    • Adam Simpkins's avatar
      fix the definition of FOLLY_HAS_STRING_VIEW on Windows · 4c227db0
      Adam Simpkins authored
      Summary:
      Update the check in `folly/Portability.h` to handle more recent versions of
      MSVC that do support `__has_include`.  The code on the `__has_include` code
      path was checking the value of `__cplusplus` rather than `_MSVC_LANG`.
      Microsoft only defines `__cplusplus` correctly when the compiler is invoked
      with the `/Zc:__cplusplus` flag.
      
      This updates the code to use the `FOLLY_CPLUSPLUS` macro instead.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26769080
      
      fbshipit-source-id: e40cfaef967b13d2fe1be2b40a109cb0ed39515a
      4c227db0
    • Dan Melnic's avatar
      Workaround for a clang -Wreorder issue · 618db6a4
      Dan Melnic authored
      Summary: Workaround for: "folly/Range.h:252:9: error: field 'b_' will be initialized after field 'e_' [-Werror,-Wreorder]"
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26788131
      
      fbshipit-source-id: 3664ea96adc26b0f97ad8d11259b018c39254692
      618db6a4
    • Maged Michael's avatar
      DynamicBoundedQueue: Round threshold up to avoid threshold 0 · 63c11c4e
      Maged Michael authored
      Summary: Ensure that threshold is not 0, otherwise when capacity < 10, threshold is 0, which can lead to consumers never transferring credit to producers.
      
      Differential Revision: D26791831
      
      fbshipit-source-id: 741bcf17de18198e43471d4b36d98c79288652fb
      63c11c4e
    • Andrew Gallagher's avatar
      Suppress unused parameter warnings on macos · 626e5caf
      Andrew Gallagher authored
      Summary: These cause unused variable warnings on other platforms.
      
      Reviewed By: mzlee
      
      Differential Revision: D26771614
      
      fbshipit-source-id: 9170c0da97375682624ee73fe422f34860f35a80
      626e5caf
    • Andrew Gallagher's avatar
      Suppress deprecation warnings on Apple platforms · a86f7668
      Andrew Gallagher authored
      Summary:
      On Apple platforms, some of the ucontext methods used here are
      deprecated.
      
      Reviewed By: yfeldblum, snufkinsnorka
      
      Differential Revision: D26765225
      
      fbshipit-source-id: 92c7e554b1d1e98b7b8566079a0cb3a0e8afc2d3
      a86f7668
    • Andrii Grynenko's avatar
      Patch AsyncStackTest.MixedStackWalk to work with the new <coroutine> header · 12593739
      Andrii Grynenko authored
      Summary: Depending on inlining of std library primitives (e.g. coroutine_handle) it's possible to have more frames in the stack trace.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26793792
      
      fbshipit-source-id: 44fb1344353e29ec92559ee34740157373112f8f
      12593739
    • Jason Rahman's avatar
      Fix incorrect purging in MemoryIdler · 64d8f3fd
      Jason Rahman authored
      Summary:
      When the given deadline to futexWaitUntil() is less than the idle
      timeout that controls purging, MemoryIdler should not purge JEMalloc arenas.
      Fix a bug where if the deadline is less than the idle timeout, purging happens
      100% of the time unconditionally.
      
      Reviewed By: mogeb, davidtgoldblatt, jalatif
      
      Differential Revision: D26784786
      
      fbshipit-source-id: 316ef96658d0cc2263b973b0b7c345605784eb36
      64d8f3fd
  6. 03 Mar, 2021 7 commits
    • Andrew Krieger's avatar
      dynamic constructor from anything String-like · e37262d4
      Andrew Krieger authored
      Summary:
      With the exception of std::string, nothing else string-like can be
      optimized further than a copy. So, create a generic Stringish constructor
      that copies .size() elements from .data(), where .data() has a value type
      of char.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5407445
      
      fbshipit-source-id: acf709f07e73bb5bef3f124169c81d76edefa52c
      e37262d4
    • Nick Terrell's avatar
      Drop support for zstd versions < 1.4.0 · e80c094b
      Nick Terrell authored
      Summary:
      Dropping support for earlier zstd versions allows us to remove all of the version check logic.
      Zstd-1.4.0 was released in April 2019, and it stabilized a large portion of our API.
      
      Reviewed By: yfeldblum, Cyan4973
      
      Differential Revision: D26585060
      
      fbshipit-source-id: 3e8bd3aa1930c993cc0f2e8fc147db66de9dfdda
      e80c094b
    • Kenny Yu's avatar
      Add gdb script to print async stack trace · c1406fae
      Kenny Yu authored
      Summary:
      This modifies the existing `co_bt` gdb script to make use of the new async stack traces available with folly::coro. This adds 2 new commands to gdb:
      
      - `co_bt` with no arguments print the async stack trace for the current thread if there is an async operation in progress. This will print the normal stack frames interleaved with async stack frames as appropriate.
      - `co_async_stack_roots` will print the `AsyncStackRoot` pointers available on the current thread
      - `co_bt [ADDRESS]` will print the async stack frames starting at the provided `AsyncStackRoot` pointer
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D26719625
      
      fbshipit-source-id: 8edf6ab0851ab92614b3dbeafdf50a517dcc3a61
      c1406fae
    • Andrii Grynenko's avatar
      Make Coroutine.h compatible with libstdc++ · ffd51f40
      Andrii Grynenko authored
      Summary: Add support for both <coroutine> and <experimental/coroutine> headers. Also add support for __cpp_impl_coroutine (which is required by the standard).
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26745085
      
      fbshipit-source-id: 0bf932e2e30d5b105b1559d817563946fcd3b573
      ffd51f40
    • Yedidya Feldblum's avatar
      let blocking_wait be insusceptible to ADL · 30ba5616
      Yedidya Feldblum authored
      Summary: As a standing rule, let us not have our things be susceptible to ADL except when we very consciously and very specifically want an extension point.
      
      Reviewed By: aary
      
      Differential Revision: D26678138
      
      fbshipit-source-id: 9f845dbc5ed95aaecf4d6cd256aedd0b240e00d3
      30ba5616
    • Andrii Grynenko's avatar
      Use coro::* instead of std::experimental::* · 02177495
      Andrii Grynenko authored
      Summary: Depending on the version of the std library, coro types may be in std or std::experimental namespace. Use coro namespace instead to make switching library implementation easier.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26744795
      
      fbshipit-source-id: 8856b901d4d757bf378acb56f236135feb0c5488
      02177495
    • Luca Niccolini's avatar
      disable --allow-system-packages for generate-github-actions · 6aa5e27b
      Luca Niccolini authored
      Summary: Fix github actions builds for projects depending on zstd
      
      Reviewed By: wez
      
      Differential Revision: D26743251
      
      fbshipit-source-id: a3fd8a14750227d025bff763cca8aa683b26a422
      6aa5e27b
  7. 02 Mar, 2021 2 commits
    • Philip Pronin's avatar
      protect executor callback destruction · 0ee7dfe1
      Philip Pronin authored
      Summary:
      Callback destruction can involve user logic.  In practice, we
      found quite a bunch of patterns like:
      
      ```
      executor->add([x = someGuardObjectThatTriggersWorkInDtor] {
        ...
      });
      ```
      
      So ensure that dtor is
      
      * Executed under `RequestContextScopeGuard`,
      * Protected from leaking unhandled exceptions.
      
      Reviewed By: ot, luciang
      
      Differential Revision: D26744394
      
      fbshipit-source-id: 5fcca0287a98de919c3649a9613e6d54cc1a1ba3
      0ee7dfe1
    • Yingnan Li's avatar
      Use ExecutionObserver in EventBase to monitor function not executed... · 204ae3a6
      Yingnan Li authored
      Use ExecutionObserver in EventBase to monitor function not executed EventBaseAtomicNotificationQueueby in EventBase
      
      Summary:
      Our service schedules tasks (eSemiFuture/coroutine) on SerialExecutor on top of IOThreadPoolExecutor. When using EventBaseWatchdog to locate long running tasks, I found some long running tasks were not observed by PerThreadExecController. After some investigation, I found there are 2 cases in EventBase that tasks are not executed by handlerReady in EventBaseAtomicNotificationQueue.
      
      1. Short-circuit scenario in runInEventBaseThread which bypasses EventBaseAtomicNotificationQueue.
      2. If notification Queue is marked 'internal', some events may be ran "manually" in loopBody.
      
      As a result, the observer logic in EventHandler (EventBaseAtomicNotificationQueue derives from EventHandler) is passed.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D26563459
      
      fbshipit-source-id: e2741cff373c89a5cd438904f910b23a64fad653
      204ae3a6
  8. 01 Mar, 2021 4 commits
    • Brandon Schlinker's avatar
      ByteEvent (socket timestamp) foundation · 842ecea5
      Brandon Schlinker authored
      Summary:
      Adding support for write and socket timestamps by introducing `ByteEvent` that can be delivered to observers.
      `AsyncTransport::WriteFlags` has long had timestamping related flags, such as `TIMESTAMP_TX`, but the code required to act on these flags only existed in proxygen. This diff generalizes the approach so that it works for other use cases of `AsyncSocket`.
      
      This diff is long, but much of it is unit tests designed to prevent regressions given the trickiness of socket timestamps and `ByteEvent`.
      
      **Each `ByteEvent` contains:**
      - Type (WRITE, SCHED, TX, ACK)
      - Byte stream offset that the timestamp is for (relative to the raw byte stream, which means after SSL in the case of AsyncSSLSocket)
      - `steady_clock` timestamp recorded by AsyncSocket when generating the `ByteEvent`
      - For SCHED, TX, and ACK events, if available, hardware and software (kernel) timestamps
      
      **How `ByteEvent` are used:**
      - Support is enabled when an observer is attached with the `byteEvents` config flag set. If the socket does not support timestamps, the observer is notified through the `byteEventsUnavailable` callback. Otherwise, `byteEventsEnabled` is called
      - When the application writes to a socket with `ByteEvent` support enabled and a relevant `WriteFlag`, SCHED/TX/ACK `ByteEvent` are requested from the kernel, and WRITE `ByteEvent` are generated by the socket for the *last byte* in the write.
        - If the entire write buffer cannot be written at once, then additional `ByteEvent` will also be generated for the last byte in each write.
        - This means that if the application wants to timestamp a specific byte, it must break up the write buffer before handing it to `AsyncSocket` such that the byte to timestamp is the last byte in the write buffer.
      - When socket timestamps arrive from the kernel via the socket error queue, they are transformed into `ByteEvent` and passed to observers
      
      **Caveats:**
      1. Socket timestamps received from the kernel contain the byte's offset in the write stream. This counter is a `uint32_t`, and thus rolls over every ~4GB. When transforming raw timestamp into `ByteEvent`, we correct for this and transform the raw offset into an offset relative to the raw byte offset stored by `AsyncSocket` (returned via `getRawBytesWritten()`).
      2. At the moment, a read callback must be installed to receive socket timestamps due to epoll's behavior. I will correct this with a patch to epoll, see https://github.com/libevent/libevent/issues/1038#issuecomment-703315425 for details
      3. If a msghdr's ancillary data contains a timestamping flag (such as `SOF_TIMESTAMPING_TX_SOFTWARE`), the kernel will enqueue a socket error message containing the byte offset of the write ( `SO_EE_ORIGIN_TIMESTAMPING`) even if timestamping has not been enabled by an associated call to `setsockopt`. This creates a problem:
          1. If an application was to use a timestamp `WriteFlags` such as `TIMESTAMP_TX` without enabling timestamping, and if `AsyncSocket` transformed such `WriteFlags` to ancillary data by default, it could create a situation where epoll continues to return `EV_READ` (due to items in the socket error queue), but `AsyncSocket` would not fetch anything from the socket error queue.
          2. To prevent this scenario, `WriteFlags` related to timestamping are not translated into msghdr ancillary data unless timestamping is enabled. This required adding a boolean to `getAncillaryData` and `getAncillaryDataSize`.
      
      Differential Revision: D24094832
      
      fbshipit-source-id: e3bec730ddd1fc1696023d8c982ae02ab9b5fb7d
      842ecea5
    • Brandon Schlinker's avatar
      netops::Dispatcher · 220215e9
      Brandon Schlinker authored
      Summary:
      Wrapper around `folly::netops` methods that makes it easier to mock these methods in unit tests.
      
      When we want to mock out calls to `folly::netops` we currently:
      - Use methods like `getSockOptVirtual` and `setSockOptVirtual`
      - Mock part of the socket, like in the tests in `AsyncSSLSocketWriteTest`
      
      I've the latter makes the tests particularly error prone, since we're mocking the object that's also under test.
      
      This change introduces `netops::Dispatcher`, which is a class containing all of the functions in `folly::netops`:
      - By default `AsyncSocket` uses a default, static instance of `Dispatcher` that forwards calls to the original `netops::` calls (e.g., calling `netops::Dispatcher::sendmsg` results in a call to `netops::sendmsg`.
      - When a test wants to mock a a `folly::netops` call, it can call `setOverrideNetOpsDispatcher` to insert a mock `netops::Dispatcher`. I use it in this manner in D24094832
      
      Differential Revision: D24661160
      
      fbshipit-source-id: e9cb4ed28ffe409c74998a1c9501c0706fc853e0
      220215e9
    • Brandon Schlinker's avatar
      Track rawBytesWritten · 8ea0a28b
      Brandon Schlinker authored
      Summary:
      During the wangle accept process and in a few pieces of application code we transform one type of `AsyncSocket` to another, potentially *after* a write has occurred.
      
      Since we do not currently carry `appBytesWritten` and `rawBytesWritten` during transformations, those values may not represent all bytes written. Likewise, if we transform from a `AsyncSSLSocket`, we lose count of the number of raw bytes written.
      
      It's very difficult to reason about whether these problems will actually manifest, so I'd prefer to just guard against it.
      
      With this change, we explicitly track the number of bytes written to the socket by incrementing a counter in `sendSocketMessage`, which is used by both `AsyncSocket` and `AsyncSSLSocket`. In addition, we copy the appBytesWritten and rawBytesWritten during socket moves.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24551958
      
      fbshipit-source-id: 88416114b52931ff3ceef847401d556ccf0ab664
      8ea0a28b
    • Brandon Schlinker's avatar
      Unify socket message generation, sendSocketMessage · 4c2bc928
      Brandon Schlinker authored
      Summary: Both `AsyncSocket` and `AsyncSSLSocket` currently have code to generate a socket message and control messages with ancillary data. Merge this code into a new function in `AsyncSocket`.
      
      Differential Revision: D24096351
      
      fbshipit-source-id: 87d90648c10c87832f868e322acf59c97b8ac8b7
      4c2bc928
  9. 28 Feb, 2021 1 commit
    • Yedidya Feldblum's avatar
      move ready_awaitable, variant_awaitable to coroutine header · bb168bd8
      Yedidya Feldblum authored
      Summary:
      Move `ready_awaitable`, `variant_awaitable` types to `folly/experimental/coro/Coroutine.h`, which reexports the foundational vocabulary coroutine helper types and is a suitable place for these vocabulary coroutine helper types.
      
      `ready_awaitable` is an awaitable which is ready and has a value or, if of void, which is just ready. `variant_awaitable` is an awaitable composed of one of several possible backing awaitables.
      
      Reviewed By: aary
      
      Differential Revision: D26270528
      
      fbshipit-source-id: e98f64b3d3ffaf5bbd322397a41bb5fe85a6bb55
      bb168bd8
  10. 27 Feb, 2021 2 commits
    • Victor Zverovich's avatar
      Deprecate folly::format · 4e249e08
      Victor Zverovich authored
      Summary:
      `folly::format` is a problematic API because it returns a `Formatter` object that may store references to arguments as its comment warns:
      
      ```
       * Formatter class.
       *
       * Note that this class is tricky, as it keeps *references* to its lvalue
       * arguments (while it takes ownership of the temporaries), and it doesn't
       * copy the passed-in format string. Thankfully, you can't use this
       * directly, you have to use format(...) below.
      ```
      
      This has negative safety and performance (encourages reuse of the same formatter) implications because contrary to what the comment says you can use the object directly since it's returned from `folly::format`. For example
      ```
      auto f = folly::format(std::string("{}"), 42);
      f.str();
      ```
      is a UB with no compile-time diagnostic (only caught by ASAN).
      
      It's also unnecessary because the `Formatter` object is usually converted to string straight away via `str()` or written to an output stream. Reusing the formatter doesn't make much sense either because the expensive part is formatting, not capturing arguments.
      
      This diff deprecates `folly::format` suggesting `fmt::format` as a potential replacement. `fmt::format` doesn't have the above problem because arguments are always in scope. It also has the following advantages:
      
      * Better compile times.
      * Compile-time format string checks.
      * Compatibility with C++20 `std::format`.
      * Better performance, particularly with format string compilation which eliminates format string processing overhead altogether.
      * Smaller binary footprint.
      
      Also remove `folly::writeTo` which is no longer used and the `format_nested_fbstrings` benchmark which is identical to `format_nested_strings` since there is no `fbstr()` any more.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D26391489
      
      fbshipit-source-id: f0309e78db0eb6d8c22b426d4cc333a17c53f73e
      4e249e08
    • Yedidya Feldblum's avatar
      rename co_invoke_type to co_invoke_fn · 2e670e0a
      Yedidya Feldblum authored
      Summary: Follow a convention that function object types with corresponding function object instances be named with a `_fn` suffix.
      
      Reviewed By: ericniebler
      
      Differential Revision: D26674781
      
      fbshipit-source-id: f8a3b8f18677c905897da8bae38cc76693146073
      2e670e0a