[PATCH] D38806: DepthFirstIterator.h: Use C++11 features to call a completed method onthe set type, instead of requiring that one exists.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin marked 2 inline comments as done.
dberlin added inline comments.


================
Comment at: include/llvm/ADT/DepthFirstIterator.h:83
+template <typename T, typename NodeRef>
+auto call_completed_method_imp(T &t, NodeRef V, bool) -> decltype(void()) {
+  return;
----------------
dblaikie wrote:
> I'm guessing this is for consistency? But I'd probably write this with a traditional return type?
> 
>   void call_completed_method_imp(...) {
yes, it was consistency, i've changed it.



================
Comment at: include/llvm/ADT/DepthFirstIterator.h:88-89
+template <typename T, typename NodeRef>
+auto call_completed_method(T &t, NodeRef V)
+    -> decltype(call_completed_method_imp(t, V, 0), void()) {
+  call_completed_method_imp(t, V, 0);
----------------
dblaikie wrote:
> What's the delayed return type achieve here - given that there's the fallback overload of call_completed_method_imp, it wouldn't be providing any SFINAE, would it? (& there's not some other overload of call_completed_method that'd be selected)
> 
> Should this be:
> 
>   void call_completed_method(...) {
>     return call_completed_method_imp(t, V, 0);
>   }
Nothing now.
To give some context, i was playing around with trying to transform the existence/non-existence into a bool template parameter that was inferred, so we wouldn't need the overload hack.
It turns out to be possible using constexpr functions, but doesn't seem to work in all compilers yet.


I've fixed this


https://reviews.llvm.org/D38806