RFR: 8251274: Provide utility types for function SFINAE using extra template parameters

[Erik Österlund]

Hi Kim,

I have one comment on this. IMO using value or type based SFINAE is 
mostly a matter of preference. Either you prefer passing in trait types, 
or you prefer passing in constexpr booleans. The constexpr boolean 
approach seems to be strictly more useful IMO, because you can pass in 
evaluated constexpr functions, which is simply not possible in type 
based SFINAE. I have for example a situation where I have VM_Version 
support for a feature being returned in a constexpr function, that I can 
use to SFINAE away platform code on unsupported platforms (actual use 
case for me).

Therefore, instead of having both EnableIfX and EnableIfT, I wonder if 
we could just stick to one convention instead, and say we pass in 
booleans, not types wrapping booleans. That way, there will never be any 
question as to which convention is being used or which one you should 
use - we use only one, and simply convert types to booleans before 
passing in to EnableIf. And then we don't need to have multiple EnableIf 
macros to differentiate the two cases. We just stick to one by 
convention and say this is the way we do things. Today we do not to my 
knowledge use any type SFINAE in HotSpot, it's all value based (the 
parameter to EnableIf is a bool, not a type). So it would further more 
seem that is the convention we already have today, and I think is the 
convention we want to keep on using, especially due to constexpr making 
that choice strictly more useful.

So basically, my feedback is that I think you can safely remove 
EnableIfT and rename EnableIfX to EnableIf instead.

What do you think?

Thanks,
/Erik

On 2020-08-07 10:33, Kim Barrett wrote:
> Please review this change which adds some utility alias templates for
> function template SFINAE via additional non-type template parameters with
> default values.  These aliases can also be used for class template SFINAE
> with a small change to current usage.
>
> With C++98/03 there are two ways to use enable_if with function templates:
>
> (1) As the return type
> (2) As an extra parameter
>
> Both are syntactically messy, polluting the function signature.
>
> C++11 adds a third way, using an extra anonymous non-type template parameter
> with a default value, i.e.
>
>    // This overload is only considered when T is an integral type.
>    template<typename T, std::enable_if_t<std::is_integral<T>::value, int> = 0>
>    void foo(T x) { ... }
>
> There's still a lot of syntactic noise in that, but we can substantially
> reduce the noise using alias templates.  With the proposed utilities, that
> can be written as
>
>    // This overload is only considered when T is an integral type.
>    template<typename T, EnableIfT<std::is_integral<T>> = 0>
>    void foo(T x) { ... }
>
> The trailing " = 0" is a small remaining syntactic annoyance.  That can't be
> eliminated without the use of macros though.  While I like macros, this
> didn't seem to me to rise to the level of being worth using them.
>
> There are four new aliases:
>
> template<bool V> EnableIfX;          // int if V, else error.
> template<typename T> EnableIfT;      // int if T::value, else error.
> template<typename... T> EnableIfAnd; // int if Conjunction<T...>::value, else error.
> template<typename... T> EnableIfOr;  // int if Disjunction<T...>::value, else error.
>
> I'm open to bikeshedding on the names of EnableIfX and EnableIfT.
>
> (I'm disappointed that there's even a need for the distinction between
> EnableIfX and EnableIfT.  A macro can completely hide that distinction
> (along with the conventional " = 0" default value if desired).
> Unfortunately, the implementation of such a macro foundered on MSVC bugs
> that I was unable to work around.  To be fair, my preferred implementation
> also ICE'd gcc.)
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8251274
>
> Webrev:
> https://cr.openjdk.java.net/~kbarrett/8251274/open.00/
>
> Testing:
> tier1, along with conversion of some existing code to use the new mechanism.
>