On Aug 22, 2017, at 9:17 PM, JF Bastien <jfb@chromium.org> wrote:
I'd suggest considering what it'll look like when we're migrating to concepts in C++20.
Here's an example for our bitwise_cast: https://github.com/jfbastien/bit_cast/blob/master/bit_cast.h#L10 <https://github.com/jfbastien/bit_cast/blob/master/bit_cast.h#L10>
Notice the 3 ways to enable. There's also the option of using enable_if on the return value, or as a defaulted function parameter, but I'm not a huge fan of either.
I think the concepts approach is the cleanest. I’d avoid the macro if we go that way. But C++20 is a long way away and I only expect this problem to get worse over time. So I’d rather find a nearer term solution.
On Aug 22, 2017, at 9:13 PM, Chris Dumez <cdumez@apple.com> wrote:
I personally prefer std::enable_if<>. For e.g.
template<typename T, class = typename std::enable_if<std::is_same<T, int>::value> Class Foo { }
I just find this much harder to parse since I now have to: 1) recognize that the last class is not a actually polymorphic parameter 2) figure out exactly what the condition is given that it’s hidden inside an enable_if* The plus side of using a static_assert based approach is that it doesn’t impact the readability of function/class signature at a high level since it’s nested inside the body. It’s also not hidden particularly hidden since I would expect it to be the first line of the body Another downside of enable_if as a default template parameter is that someone could make a mistake and pass an extra template value, e.g. Foo<float, int>, and it might pick the wrong template parameter. This isn’t super likely but it’s still a hazard. Admittedly, we could make a macro like (totes not stolen from JF’s GitHub): #define ENABLE_TEMPLATE_IF(condition) typename = typename std::enable_if<condition>::type and implement Foo as: template<typename T, ENABLE_TEMPLATE_IF(std::is_same<T, int>::value)> class Foo { }; I think this approach is pretty good, although, I think I care about the enable_if condition rarely enough that I’d rather not see it in the signature. Most of the time the code will look like: template<typename T, ENABLE_TEMPLATE_IF(std::is_same<T, int>::value)> class Foo {...}; template<typename T, ENABLE_TEMPLATE_IF(std::is_same<T, float>::value)> class Foo {...}; template<typename T, ENABLE_TEMPLATE_IF(std::is_same<T, double>::value)> class Foo {...}; So when I know I want to use a Foo but I forgot the signature I now need to look mentally skip the enable_if macro, which I’d rather avoid.
I don’t like that something inside the body of a class / function would cause a template to be enabled or not.
I believe there are cases where this already basically already happens e.g. bitwise_cast. Although, I think those cases could be fixed with a more standard approach. Cheers, Keith
-- Chris Dumez
On Aug 22, 2017, at 8:34 PM, Keith Miller <keith_miller@apple.com <mailto:keith_miller@apple.com>> wrote:
Hello fellow WebKittens,
I’ve noticed over time that we don’t have standard way that we enable versions of template functions/classes (flasses?). For the most part it seems that people use std::enable_if, although, it seems like it is attached to every possible place in the function/class.
I propose that we choose a standard way to conditionally enable a template.
There are a ton of options; my personal favorite is to add the following macro:
#define ENABLE_TEMPLATE_IF(condition) static_assert(condition, “template disabled”)
Then have every function do:
template<typename T> void foo(…) { ENABLE_TEMPLATE_IF(std::is_same<T, int>::value); … }
And classes:
template<typename T> class Foo { ENABLE_TEMPLATE_IF(std::is_same<T, int>::value); };
I like this proposal because it doesn’t obstruct the signature/declaration of the function/class but it’s still obvious when the class is enabled. Obviously, I think we should require that this macro is the first line of the function or class for visibility. Does anyone else have thoughts or ideas?
Cheers, Keith
P.S. in case you are wondering why this macro works (ugh C++), it’s because if there is any compile time error in a template it cannot be selected as the final candidate. In my examples, if you provided a type other than int foo/Foo could not be selected because the static_assert condition would be false, which is a compile error. _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org <mailto:webkit-dev@lists.webkit.org> https://lists.webkit.org/mailman/listinfo/webkit-dev