[flang] Use more generic overload for Operation in Traverse

[kparzysz]

Currently there are two specific overloads: for unary operations, i.e. Operation<D, R, O>, and binary ones Operation<D, R, LO, RO>.

This makes it impossible for a derived class to use a single overload to handle all types of operations: Operation<D, R, O...>. Since the base overloads need to be included in the derived class's scope, via using Base::operator() either one of the specific overloads will always be a better candidate than the more generic derived one.

class MyVisitor : public Traverse<...> {
  using Traverse<...>::operator();

  template <typename D, typename R, typename... O>
  Result operator()(const Operation<D, R, O...> &op) const {
    // Will never be used.
  }
};

This patch replaces the two specific overloads for Operation in Traverse with a single generic overload, while preserving the existing functionality, and allowing derived classes to use a single overload as well.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

Currently there are two specific overloads: for unary operations, i.e. Operation<D, R, O>, and binary ones Operation<D, R, LO, RO>.

This makes it impossible for a derived class to use a single overload to handle all types of operations: Operation<D, R, O...>. Since the base overloads need to be included in the derived class's scope, via using Base::operator() either one of the specific overloads will always be a better candidate than the more generic derived one.

class MyVisitor : public Traverse<...> {
  using Traverse<...>::operator();

  template <typename D, typename R, typename... O>
  Result operator()(const Operation<D, R, O...> &op) const {
    // Will never be used.
  }
};

This patch replaces the two specific overloads for Operation in Traverse with a single generic overload, while preserving the existing functionality, and allowing derived classes to use a single overload as well.

---

Full diff: https://github.com/llvm/llvm-project/pull/133305.diff

1 Files Affected:

• (modified) flang/include/flang/Evaluate/traverse.h (+15-7)

diff --git a/flang/include/flang/Evaluate/traverse.h b/flang/include/flang/Evaluate/traverse.h
index 9bb677e515372..45402143604f4 100644
--- a/flang/include/flang/Evaluate/traverse.h
+++ b/flang/include/flang/Evaluate/traverse.h
@@ -227,13 +227,14 @@ class Traverse {
   }
 
   // Operations and wrappers
-  template <typename D, typename R, typename O>
-  Result operator()(const Operation<D, R, O> &op) const {
-    return visitor_(op.left());
-  }
-  template <typename D, typename R, typename LO, typename RO>
-  Result operator()(const Operation<D, R, LO, RO> &op) const {
-    return Combine(op.left(), op.right());
+  // Have a single operator() for all Operations.
+  template <typename D, typename R, typename... Os>
+  Result operator()(const Operation<D, R, Os...> &op) const {
+    if constexpr (sizeof...(Os) == 1) {
+      return visitor_(op.left());
+    } else {
+      return CombineOperands(op, std::index_sequence_for<Os...>{});
+    }
   }
   Result operator()(const Relational<SomeType> &x) const {
     return visitor_(x.u);
@@ -269,6 +270,13 @@ class Traverse {
     return CombineRange(x.begin(), x.end());
   }
 
+  template <typename D, typename R, typename... Os, size_t... Is>
+  Result CombineOperands(
+      const Operation<D, R, Os...> &op, std::index_sequence<Is...>) const {
+    static_assert(sizeof...(Os) > 1 && "Expecting multiple operands");
+    return Combine(op.template operand<Is>()...);
+  }
+
   template <typename A, typename... Bs>
   Result Combine(const A &x, const Bs &...ys) const {
     if constexpr (sizeof...(Bs) == 0) {