> On Oct. 1, 2014, 9:30 p.m., Dominic Hamon wrote: > > src/slave/status_update_manager.cpp, line 570 > > <https://reviews.apache.org/r/26247/diff/1/?file=710348#file710348line570> > > > > this is an unfortunate pattern. I assume your changes in dispatch have > > somehow changed the template deduction so this is required everywhere we > > dispatch. > > > > can you identify what that change is and consider reverting it? This is > > a regression for the API. > > Michael Park wrote: > The change that requires this is what's described in the `Description` > section. But here's a full overview. > > ## Preprocessor Expansion: > > ```cpp > // '&F' by itself is ambiguous because 'F' is an overloaded function. > void F(int) {} > void F(int, double) {} > > // Snippet of what could be generated by preprocessor expansion. > > template <typename P, typename A> > void G(void (*)(P), A) {} > > template <typename P0, typename P1, typename A0, typename A1> > void G(void (*)(P0, P1), A0, A1) {} > > // ... > > int main() { > // '&F' is also unambiguous thanks to the number of arguments. > G(&F, 42); > G(&F, 42, 1.1); > } > ``` > > ## Variadic Template: > > ```cpp > // '&F' by itself is ambiguous because 'F' is an overloaded function. > void F(int) {} > void F(int, double) {} > > template <typename... Ps, typename... As> > void G(void (*)(Ps...), As...) {} > > int main() { > // '&F' can no longer be disambiguated by the number of arguments. > G(static_cast<void (*)(int)>(&F), 42); > G(static_cast<void (*)(int, double)>(&F), 42, 1.1); > } > ``` > > However, this method of disambiguating an overloaded function is > unreliable because it's not an inherant property of overloaded functions to > have different lengths. For example, consider this example with the > preprocessor expansion: > > ```cpp > // '&F' by itself is ambiguous because 'F' is an overloaded function. > void F(int) {} > void F(double) {} > > // Snippet of what could be generated by preprocessor expansion. > > template <typename P, typename A> > void G(void (*)(P), A) {} > > template <typename P0, typename P1, typename A0, typename A1> > void G(void (*)(P0, P1), A0, A1) {} > > // ... > > int main() { > // Need to explicitly disambiguate '&F'. > G(static_cast<void (*)(int)>(&F), 42); > G(static_cast<void (*)(double)>(&F), 1.1); > } > ```
Other than moving to native C++11 features, do we gain anything else with this change? Are compile times significantly faster? As Dominic points out, this is definitely a regression in the API. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26247/#review55140 ----------------------------------------------------------- On Oct. 5, 2014, 7:08 a.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26247/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2014, 7:08 a.m.) > > > Review request for mesos and Dominic Hamon. > > > Repository: mesos-git > > > Description > ------- > > These are changes necessary from removing `<stout/preprocessor.hpp>` from > `stout` and `libprocess`. > With the preprocessor expansion, taking the address of an overloaded function > was disambiguated by the number of arguments. > The number of arguments a function takes is not a reliable disambiguation > mechanism, we `static_cast` them to a specific signature as we do in other > parts of the code where disambiguation of overloaded functions is necessary. > > > Diffs > ----- > > src/log/log.cpp b3c6c20d734ea82d8531217d53decf1707183565 > src/log/replica.cpp c18de86f2659b848b0e8f0468e4efa820d048970 > src/master/allocator.hpp 02d20d0cc802805bc702891306aa42894531b223 > src/scheduler/scheduler.cpp fb88a3e029e97ba33eae5d50503be5ed9c9533e6 > src/slave/containerizer/composing.cpp > 9022700b628d9746a6a8a17c9fbf1b1988da6fca > src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 > src/slave/containerizer/mesos/containerizer.cpp > 9d083294caa5c5a47ba3ceaa1b57346144cb795c > src/slave/status_update_manager.cpp > 5d5cf234ef2dd47fa4b1f67be761dbca31659451 > src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 > src/zookeeper/zookeeper.cpp d4c24cd500b74d3b979a471b4a32def78958f04a > > Diff: https://reviews.apache.org/r/26247/diff/ > > > Testing > ------- > > `make && make check` on `gcc-4.4`, `gcc-4.6`, `gcc-4.8`, `gcc-4.9`, > `clang-3.3` and `clang-3.5` > > > Thanks, > > Michael Park > >
