You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Daniel Pravat <dp...@outlook.com> on 2015/12/01 21:38:13 UTC

Re: Review Request 40114: Windows: Began adding Windows support to `process/future.hpp`

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40114/
-----------------------------------------------------------

(Updated Dec. 1, 2015, 8:38 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.


Repository: mesos


Description
-------

Added Windows & Posix defines for several parameter list used in future.hpp. 
Provided a new parameter list for VC2015.
Move the existing parameter list into posix.hpp. 

The change has been due to: 

   1. VS 2015 won't support C++14 result_of SFINAE until Update 2,  so
      result_of must be replaced with decltype(invoke).
   2. VS 2015 won't support C++14 std::function SFINAE until Update 2, so
      converting _Deferred to std::function must be done by explicitly
      calling _Deferred's conversion function.
   3. The Standard (C++11 through 17) does not require bind's function call
      operator to SFINAE, and VS 2015's doesn't.  is_bind_expression can be
      used to manually reroute bind expressions to the 1-arg overload, where
      (conveniently) the argument will be ignored if necessary.


Diffs (updated)
-----

  3rdparty/libprocess/include/Makefile.am 47f5347988a61140c87bcd329e25d5a4d52e17a0 
  3rdparty/libprocess/include/process/future.hpp c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 
  3rdparty/libprocess/include/process/posix/future.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/windows/future.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/40114/diff/


Testing
-------

OSX make check, Windows 10 make, Ubuntu 15.1 make check


Thanks,

Daniel Pravat


Re: Review Request 40114: Windows: Began adding Windows support to `process/future.hpp`

Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40114/#review110382
-----------------------------------------------------------


Thanks for the detailed analysis of the issues!

(1) My preference here is to add a `result_of` to stout with C++14 semantics until VS 2015 Update 2,
    at which point we can simply swap it back with `s/result_of/std::result_of/`.
(2) Let's pull this part into a separate, trivial patch
(3) We should really also do this in the posix version, since as STL points out, it's not required by the standard.
    Let's also pull this part out into a separate patch

- Michael Park


On Dec. 1, 2015, 8:38 p.m., Daniel Pravat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40114/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 8:38 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Windows & Posix defines for several parameter list used in future.hpp. 
> Provided a new parameter list for VC2015.
> Move the existing parameter list into posix.hpp. 
> 
> The change has been due to: 
> 
>    1. VS 2015 won't support C++14 result_of SFINAE until Update 2,  so
>       result_of must be replaced with decltype(invoke).
>    2. VS 2015 won't support C++14 std::function SFINAE until Update 2, so
>       converting _Deferred to std::function must be done by explicitly
>       calling _Deferred's conversion function.
>    3. The Standard (C++11 through 17) does not require bind's function call
>       operator to SFINAE, and VS 2015's doesn't.  is_bind_expression can be
>       used to manually reroute bind expressions to the 1-arg overload, where
>       (conveniently) the argument will be ignored if necessary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/Makefile.am 47f5347988a61140c87bcd329e25d5a4d52e17a0 
>   3rdparty/libprocess/include/process/future.hpp c9146e3a3ccf09dd37c5a8ac7000fbe84f3c710c 
>   3rdparty/libprocess/include/process/posix/future.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/windows/future.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40114/diff/
> 
> 
> Testing
> -------
> 
> OSX make check, Windows 10 make, Ubuntu 15.1 make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>