1. 27 Aug, 2018 4 commits
    • Dan Zimmerman's avatar
      Make memory/Arena.h work with -fno-exceptions · 1aac76b0
      Dan Zimmerman authored
      Summary: We may want to disable exceptions, so lets make this work
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9381726
      
      fbshipit-source-id: 6e6d022740728a40a5b710ec32309da99f6d0bd0
      1aac76b0
    • Dan Zimmerman's avatar
      Split out folly::enable_shared_from_this from Memory.h · 1a32e2cf
      Dan Zimmerman authored
      Summary:
      This doesn't need to be in Memory.h and is one of the reasons Memory.h fails to compile with -fno-exceptions, so split it out
      
      I include EnableSharedFromThis.h because I'm not sure how else to ensure backward compatibility any other way (and I have the macro so we can include Memory.h without including EnableSharedFromThis.h)
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9385709
      
      fbshipit-source-id: bf2c3a757ae4eefba69e6191309d64b347becf12
      1a32e2cf
    • Marshall Cline's avatar
      use rvalue-qual Future::within(...); pass 5 · c083f627
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).within(...)` instead of `f.within(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Codemod changes:
      
      * expr.within(...) ==> std::move(expr).within(...)  // if expr is not already an xvalue
      * expr->within(...) ==> std::move(*expr).within(...)
      
      Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9511943
      
      fbshipit-source-id: 0cf90e3198453dd997194c2c7f36cc1fa74906f5
      c083f627
    • Yedidya Feldblum's avatar
      invoke_noreturn_cold · 2e00dff8
      Yedidya Feldblum authored
      Summary:
      [Folly] `invoke_noreturn_cold`, a helper function for invoking another function with arguments in a `[[noreturn]]` and `[[gnu::cold]]` context.
      
      Potentially useful for making the bytecode at the call-site small for icache size or always-inlined fast-path size concerns, when constructing the exception object to be thrown is more complex than passing pre-existing values by reference, such as when computing a message is required.
      
      Reviewed By: marshallcline
      
      Differential Revision: D9509658
      
      fbshipit-source-id: b8272629876f9e7e81ae0f7a7f06404484d297f2
      2e00dff8
  2. 26 Aug, 2018 2 commits
    • Marshall Cline's avatar
      remove lvalue-qual Future::semi() · 22a5896e
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).semi()` instead of `f.semi()`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9442704
      
      fbshipit-source-id: 50b1912e33ac23d1a693682fbcd55cd637cdb149
      22a5896e
    • Yedidya Feldblum's avatar
      Cut use of boost headers in sorted_vector_types.h · eab02f06
      Yedidya Feldblum authored
      Summary: [Folly] Cut use of boost headers in `sorted_vector_types.h`.
      
      Reviewed By: elsteveogrande
      
      Differential Revision: D9508079
      
      fbshipit-source-id: 46950a07d1fbc0e316bd6a8cb9766f5d0aeaa74b
      eab02f06
  3. 25 Aug, 2018 2 commits
  4. 24 Aug, 2018 6 commits
    • Lee Howes's avatar
      Implement thenTry and thenValue in terms of thenImplementation · 27ef8b1e
      Lee Howes authored
      Summary:
      Modify the implementations of thenValue and thenError to be in terms of thenImplementation rather than then.
      
      This is a preliminary transformation before splitting then and deprecating parts of it in turn.
      
      Reviewed By: marshallcline
      
      Differential Revision: D9494305
      
      fbshipit-source-id: 38cd165866f98514ee506dc01f625b7df1db211b
      27ef8b1e
    • Marshall Cline's avatar
      remove lvalue-qual Future::thenMulti(...) · 1c32e4d3
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).thenMulti(...)` instead of `f.thenMulti(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9437627
      
      fbshipit-source-id: f62dc095ce57904057a97c83c958de5227a112a8
      1c32e4d3
    • Andrii Grynenko's avatar
      Make sure old singleton doesn't get reported as leak if make_mock is used · 2819a880
      Andrii Grynenko authored
      Differential Revision: D9499697
      
      fbshipit-source-id: b04e9e0d1bf4abf1b5ff497cefbc26da0176eaa2
      2819a880
    • Melissa Winstanley's avatar
      Add serialization option to escape specific ASCII characters · afe40e06
      Melissa Winstanley authored
      Summary: In some cases, it may be necessary to unicode-escape regular ASCII characters in JSON serialization (example: for JSON sent to browsers that may be interpreted as HTML, "<" should be escaped). Allow additional escape characters to be specified via a bitmap in the serializer options.
      
      Reviewed By: yfeldblum, luciang
      
      Differential Revision: D8980189
      
      fbshipit-source-id: 000c5279ab0f37a3ee4b2eb38f20afa49dcc5a27
      afe40e06
    • Joe Loser's avatar
      AtomicSharedPtr-detail.h does not work with libc++ (#900) · bc16b096
      Joe Loser authored
      Summary:
      Problem:
      - There are several tests enabled which rely on using
        shared_ptr_internals found in
        folly/concurrency/detail/AtomicSharedPtr-detail.h and
        the implementation only works for libstdc++ and not with other vendors
        such as libc++ and libcpp.
      - This results in a hard error when compiling with Clang with libc++.
      
      Solution:
      - Add a CMake configure check to detect whether one is using libstdc++.
        FOLLY_USE_LIBSTDCPP macro is set to 1 if using libstdc++, 0 otherwise.
      - Use FOLLY_USE_LIBSTDCPP to conditionally run the tests relying on
        shared_ptr_internals only when using libstdc++.
      
      Note:
      - A longer-term solution would be to implement the similar
        shared_ptr_internals with the libc++ types and provide a fallback
        locking approach for libcpp (MSVC standard library implementation).
      
      Pull Request resolved: https://github.com/facebook/folly/pull/900
      
      Reviewed By: djwatson
      
      Differential Revision: D9312236
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 657eddc1e530b00bd5f5b10e7155bc3b6aac3726
      bc16b096
    • Yedidya Feldblum's avatar
      Cut unnecessary uses of Super:: in F14Policy.h · 27ab332b
      Yedidya Feldblum authored
      Summary: [Folly] Cut unnecessary uses of `Super::` in `F14Policy.h`.
      
      Reviewed By: shixiao
      
      Differential Revision: D9478746
      
      fbshipit-source-id: ea5b57a66e265ea459a66ff3756a2a54326e73f2
      27ab332b
  5. 23 Aug, 2018 16 commits
    • Marshall Cline's avatar
      remove lvalue-qual Future::thenMultiWithExecutor(...) · 56fae0e5
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).thenMulti(...)` instead of `f.thenMulti(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9437767
      
      fbshipit-source-id: f40d0660a50ccfec1040e42c8028efde956a4b48
      56fae0e5
    • Yedidya Feldblum's avatar
      Work around member alias bugs in some compilers in F14Policy.h · 1de92cd7
      Yedidya Feldblum authored
      Summary:
      [Folly] Work around member alias bugs in some compilers in `F14Policy.h`.
      
      MSVC appears to have a hard time with some cases of `using typename Base::MemberAlias;` - replace with `using MemberAlias = typename Base::MemberAlias;`.
      
      Reviewed By: shixiao
      
      Differential Revision: D9478688
      
      fbshipit-source-id: 5cbf63839cc2b3a085ed633cc4e5703d61990033
      1de92cd7
    • Sargun Vohra's avatar
      Add groupByAdjacent operator to folly::gen · d3fe49a0
      Sargun Vohra authored
      Summary:
      I find myself wanting something like this pretty often, so I tried my hand at adding it myself.
      
      The `groupByAdjacent` operator creates groups bounded wherever the selector changes. It's especially useful for processing sources that've already been sorted on the selector, such as from a database query.
      
      Given the following source sequence with keys `A` and `B`:
      
      ```
      [A1, A2, A3, B1, B2, B3, A4, A5, B4, B5]
      ```
      
      a regular `groupBy` would return something like:
      
      ```
      [A:[A1, A2, A3, A4, A5], B:[B1, B2, B3, B4, B5]]
      ```
      
      while this `groupByAdjacent` would return:
      
      ```
      [A:[A1, A2, A3], B:[B1, B2, B3], A:[A4, A5], B:[B4, B5]]
      ```
      
      Given a source where the items are presorted by selector, `groupByAdjacent` should behave identically to `groupBy`, except that `groupByAdjacent` supports infinite sources since it doesn't need to collect the entire source in memory before creating any output.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9475326
      
      fbshipit-source-id: 1c8db3abadce5e68394e5fa38bf4bee0b413a03f
      d3fe49a0
    • Orvid King's avatar
      Include stdexcept in FBString · 647220dd
      Orvid King authored
      Summary:
      It is where `std::logic_error` is defined.
      
      Fixes https://github.com/facebook/folly/issues/913
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9436370
      
      fbshipit-source-id: a4425d7d29f0bd55ea536a49a8923f22944ecbd0
      647220dd
    • Xiao Shi's avatar
      use F14NodeMap with heterogeneous lookup / mutation as StringKeyedUnorderedMap|Set · 64dabe32
      Xiao Shi authored
      Summary:
      `F14NodeMap` has proven to be a safe and more performant drop-in replacement
      for `std::unordered_map`. With the ability to do heterogeneous lookup and
      mutation, we no longer need `StringKeyedUnorderedMap`, whose main purpose was
      to avoid unnecessary string copies during lookups and inserts.
      
      Reviewed By: nbronson
      
      Differential Revision: D9124737
      
      fbshipit-source-id: 81d5be1df9096ac258a3dc1523a6a0f5d6b9e862
      64dabe32
    • Ajanthan Asogamoorthy's avatar
      fix proxygen travis builds · fa72983f
      Ajanthan Asogamoorthy authored
      Summary: in the opensource spec I forgot to specify the github org as facebookincubator in D9337956 vs regular facebook, this caused travis builds for proxygen and anything that depends on fizz to fail
      
      Reviewed By: knekritz
      
      Differential Revision: D9482739
      
      fbshipit-source-id: ba5a8f465c81ec1b9f037114ce1da3614a838699
      fa72983f
    • Dan Melnic's avatar
      ThreadCachedInts use after free fix · 8438dbe1
      Dan Melnic authored
      Summary: ThreadCachedInts use after free fix
      
      Reviewed By: djwatson
      
      Differential Revision: D9445106
      
      fbshipit-source-id: 989969e54178b5f908f6ca94cde4088ffb2da490
      8438dbe1
    • Victor Zverovich's avatar
      Replace boost::scoped_array with std::unique_ptr · de030d23
      Victor Zverovich authored
      Summary:
      `boost::scoped_ptr<T>` is obsolete so replace it with `std::unique_ptr<T[]>`.
      This also fixes the modular build by eliminating problematic using directives.
      
      Reviewed By: shixiao
      
      Differential Revision: D9480193
      
      fbshipit-source-id: 8eaa642fe16e91b8f8144f5a10f21729ed984257
      de030d23
    • Marshall Cline's avatar
      use rvalue-qual Future::semi(); pass 2 · 2cafd523
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).semi()` instead of `f.semi()`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9478283
      
      fbshipit-source-id: 7d2f54a8aaa9156b2b274aafc9695fd996f29641
      2cafd523
    • Dan Melnic's avatar
      Fix the broken folly/tests/.. build · 589a11d5
      Dan Melnic authored
      Summary: Fix the broken folly/tests/.. build
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9477364
      
      fbshipit-source-id: 0b227ebb3eb7e062bff8883ba31b398672273c16
      589a11d5
    • Yedidya Feldblum's avatar
      Fix forwarding in ParkingLot<T>::WaitNode ctor · 1c0ce290
      Yedidya Feldblum authored
      Summary: [Folly] Fix forwarding in `ParkingLot<T>::WaitNode` ctor.
      
      Reviewed By: aary
      
      Differential Revision: D9478603
      
      fbshipit-source-id: 1da4c985bee7293356b1f5e1b43678282703b8ce
      1c0ce290
    • Marshall Cline's avatar
      Workaround gcc oddity with Future::within() · 513fcbdf
      Marshall Cline authored
      Summary:
      Prior to this diff, the two `Future::within(Duration, Timekeeper* = nullptr)` overloads were the same typewise but they used different symbols:
      
      Return-type differences:
      * rvalue-qualified overload returned `Future<T>`
      * lvalue-qualified overload returned `auto` (which resolved to `Future<T>`)
      
      Parameter *name* differences:
      * rvalue-qualified overload has unnamed formal-parameters (it is a declaration but not a definition).
      * lvalue-qualified overload has named formal-parameters (it is both a declaration and a definition).
      
      These seemingly innocuous differences caused gcc to choke: when the `this` object was a `Future<...>` object returned by value, gcc reported the following error: "error: call of overloaded ‘within(std::chrono::minutes)’ is ambiguous", specifically pointing to the two `Future::within(Duration, Timekeeper* = nullptr)` overloads.
      
      Conversely clang had no problem with that callsite.
      
      gcc stopped issuing the spurious error-message after this diff's changes, that is, after making the signatures of the two overloads identical in every respect except for the rvalue-qualification vs. lvalue-qualification.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9474736
      
      fbshipit-source-id: 758001090e5487bd70c9853940807cbdeba8ac94
      513fcbdf
    • Marshall Cline's avatar
      rvalueification of Future::onTimeout(...): 1/n · 55a896f5
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onTimeout(...)` instead of `f.onTimeout(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9441928
      
      fbshipit-source-id: 28cb393588fa32becb44c89c6afd0ed482f5cf7e
      55a896f5
    • Marshall Cline's avatar
      workaround gcc oddity with Future::semi() · 018a9fe6
      Marshall Cline authored
      Summary:
      Prior to this diff, the two `Future::semi()` overloads were logically the same but used different symbols:
      * rvalue-qualified overload returned `Future<T>`
      * lvalue-qualified overload returned `auto` (which resolved to `Future<T>`)
      
      This seemingly innocuous difference caused gcc to choke: when the `this` object was a `Future<...>` object returned by value, gcc reported the following error: "error: call of overloaded ‘semi()’ is ambiguous", specifically pointing to the two `Future::semi()` overloads.
      
      Conversely clang had no problem with that callsite.
      
      gcc stopped issuing the spurious error-message after this diff's changes, that is, after making the signatures of the two overloads identical in every respect except for the rvalue-qualification vs. lvalue-qualification.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9475854
      
      fbshipit-source-id: 5fb20f6c57eaf6a5f9b7cb6cdb49782e40704953
      018a9fe6
    • Lee Howes's avatar
      Remove valueCallableResult support for no parameter · 9addc8b4
      Lee Howes authored
      Summary: This was causing type inference problems for default parameter continuations, and as the callable that thenValue takes must have a parameter it serves no purpose.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D9441278
      
      fbshipit-source-id: 8a6962772a58f51c93ef95abc06acb4939bc4a95
      9addc8b4
    • Dan Melnic's avatar
      Add RcuBenchmark as an (RcuTest, Perf) replacement · db999c04
      Dan Melnic authored
      Summary: Add RcuBenchmark as an (RcuTest, Perf) replacement
      
      Reviewed By: djwatson
      
      Differential Revision: D9443217
      
      fbshipit-source-id: c751b98e0dd38128a157cfdfad6b21b87dbfb41c
      db999c04
  6. 22 Aug, 2018 7 commits
    • Xiao Shi's avatar
      disable containerSize tests for iOS platforms temporarily · 99b167c5
      Xiao Shi authored
      Summary:
      Certain toolchains on iOS platforms don't apply EBO to F14 BasePolicy and
      F14Table, which makes the size calculations incorrect.
      
      This diff disables the test pending further investigation (T33121191).
      
      Reviewed By: nbronson
      
      Differential Revision: D9467473
      
      fbshipit-source-id: 50435b3fcdeb81a6a892e280fdc919486f0ed107
      99b167c5
    • Ajanthan Asogamoorthy's avatar
      Link in fizz to wangle and wangle's dependencies · f8afdfaa
      Ajanthan Asogamoorthy authored
      Summary: Update cmake configurations + legocastle jobs in order to add fizz as a dependency to wangle
      
      Reviewed By: reanimus
      
      Differential Revision: D9337956
      
      fbshipit-source-id: 40f25694c2b3fd8aa37d254bc63a664f4c8ee526
      f8afdfaa
    • Marshall Cline's avatar
      use rvalue-qual Future::onError(); pass 2 · b30b9e62
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onError(...)` instead of `f.onError(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Codemod changes:
      
      * expr.MMM(...) ==> std::move(expr).MMM(...)  // if expr is not already an xvalue
      * expr->MMM(...) ==> std::move(*expr).MMM(...)
      
      Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9445122
      
      fbshipit-source-id: 1202bc2dd2e054b541458aa20829abe4c7f52b33
      b30b9e62
    • Michael Lee's avatar
      Remove the fallback message in the Intrinsics check · dde15a90
      Michael Lee authored
      Summary: The message is useful once, but not for every time the header is included and the condition checked.
      
      Reviewed By: shixiao
      
      Differential Revision: D9457501
      
      fbshipit-source-id: 35c2c2f0510e9be9b747ab7c760a7cc36ad500b4
      dde15a90
    • Marshall Cline's avatar
      rvalueification of Future::semi(): 1/n · 3de7d897
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).semi()` instead of `f.semi()`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9442672
      
      fbshipit-source-id: ea0e97915b8143e2f36e956be708dc36a735cca5
      3de7d897
    • Marshall Cline's avatar
      rvalueification of Future::within(...): 1/n · dcde6d37
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).within(...)` instead of `f.within(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9442173
      
      fbshipit-source-id: 910914fd9da80627b335419542b5bb358b21f680
      dcde6d37
    • Marshall Cline's avatar
      rvalueification of Future::onError(...): 1/n · 9f748fb5
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onError(...)` instead of `f.onError(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9441271
      
      fbshipit-source-id: c65614a5529bf50bb9a209e3941d20f3688f2a80
      9f748fb5
  7. 21 Aug, 2018 3 commits
    • Marshall Cline's avatar
      rvalueification of Future::thenMultiWithExecutor(...): 1/n · 79f4a50c
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      * This is something we should do for safety in general.
      * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
      * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
      * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).thenMulti(...)` instead of `f.thenMulti(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D9437727
      
      fbshipit-source-id: 7656aff65db69a5cb51b621b7d7b1b01ef0d3e12
      79f4a50c
    • Nathan Bronson's avatar
      make a few conversions in folly more explicit · 78a84e80
      Nathan Bronson authored
      Summary:
      These conversions enable a few bits of folly to compile with
      more stringent compiler warnings than the baseline.  One site actually
      had a bug in an error path, the rest are just churn.
      
      Reviewed By: sathyaphoenix
      
      Differential Revision: D9398429
      
      fbshipit-source-id: d9836eabe93be7aeebd59581df8fb6d278c74112
      78a84e80
    • Orvid King's avatar
      Workaround ICE in MSVC 2017.8 · 6dd0f369
      Orvid King authored
      Summary: MSVC is having issues with this, so explicitly refer to `AllocTraits` via `Super`.
      
      Reviewed By: wez
      
      Differential Revision: D9387914
      
      fbshipit-source-id: 1751b029d678151cbce90cdffdce586498cdf756
      6dd0f369