Skip to content

Conversation

@kparzysz
Copy link
Contributor

The parse-tree-visitor consists of a range of Walk functions where each overload is specialized for a particular case. These overloads do call one another, and due to the usual name lookup rules, an earlier overload can't call an overload defined later unless the latter was declared ahead of time.
To avoid listing a number of declarations at the beginning of the header enclose them in a class as static members, with a couple of simple forwarding calls. This takes advantage of the class member name lookup, which uses the entire class definition for lookup.

The parse-tree-visitor consists of a range of Walk functions where each
overload is specialized for a particular case. These overloads do call
one another, and due to the usual name lookup rules, an earlier overload
can't call an overload defined later unless the latter was declared
ahead of time.
To avoid listing a number of declarations at the beginning of the header
enclose them in a class as static members, with a couple of simple
forwarding calls. This takes advantage of the class member name lookup,
which uses the entire class definition for lookup.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:parser labels Nov 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

The parse-tree-visitor consists of a range of Walk functions where each overload is specialized for a particular case. These overloads do call one another, and due to the usual name lookup rules, an earlier overload can't call an overload defined later unless the latter was declared ahead of time.
To avoid listing a number of declarations at the beginning of the header enclose them in a class as static members, with a couple of simple forwarding calls. This takes advantage of the class member name lookup, which uses the entire class definition for lookup.


Patch is 49.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/115926.diff

1 Files Affected:

  • (modified) flang/include/flang/Parser/parse-tree-visitor.h (+783-746)
diff --git a/flang/include/flang/Parser/parse-tree-visitor.h b/flang/include/flang/Parser/parse-tree-visitor.h
index 81d01dbdd65cc9..e1ea4d459f4a60 100644
--- a/flang/include/flang/Parser/parse-tree-visitor.h
+++ b/flang/include/flang/Parser/parse-tree-visitor.h
@@ -29,881 +29,918 @@
 
 namespace Fortran::parser {
 
-// Default case for visitation of non-class data members, strings, and
-// any other non-decomposable values.
-template <typename A, typename V>
-std::enable_if_t<!std::is_class_v<A> || std::is_same_v<std::string, A> ||
-    std::is_same_v<CharBlock, A>>
-Walk(const A &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    visitor.Post(x);
-  }
-}
-template <typename A, typename M>
-std::enable_if_t<!std::is_class_v<A> || std::is_same_v<std::string, A> ||
-    std::is_same_v<CharBlock, A>>
-Walk(A &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    mutator.Post(x);
-  }
-}
+template <typename A, typename V> void Walk(const A &x, V &visitor);
+template <typename A, typename M> void Walk(A &x, M &mutator);
 
-template <typename V> void Walk(const format::ControlEditDesc &, V &);
-template <typename M> void Walk(format::ControlEditDesc &, M &);
-template <typename V> void Walk(const format::DerivedTypeDataEditDesc &, V &);
-template <typename M> void Walk(format::DerivedTypeDataEditDesc &, M &);
-template <typename V> void Walk(const format::FormatItem &, V &);
-template <typename M> void Walk(format::FormatItem &, M &);
-template <typename V> void Walk(const format::FormatSpecification &, V &);
-template <typename M> void Walk(format::FormatSpecification &, M &);
-template <typename V> void Walk(const format::IntrinsicTypeDataEditDesc &, V &);
-template <typename M> void Walk(format::IntrinsicTypeDataEditDesc &, M &);
-
-// Traversal of needed STL template classes (optional, list, tuple, variant)
-// For most lists, just traverse the elements; but when a list constitutes
-// a Block (i.e., std::list<ExecutionPartConstruct>), also invoke the
-// visitor/mutator on the list itself.
-template <typename T, typename V> void Walk(const std::list<T> &x, V &visitor) {
-  for (const auto &elem : x) {
-    Walk(elem, visitor);
+namespace detail {
+// A number of the Walk functions below call other Walk functions. Define
+// a dummy class, and put all of them in it to ensure that name lookup for
+// Walk considers all overloads (not just those defined prior to the call
+// to Walk).
+struct ParseTreeVisitorLookupScope {
+  // Default case for visitation of non-class data members, strings, and
+  // any other non-decomposable values.
+  template <typename A, typename V>
+  static std::enable_if_t<!std::is_class_v<A> ||
+      std::is_same_v<std::string, A> || std::is_same_v<CharBlock, A>>
+  Walk(const A &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      visitor.Post(x);
+    }
   }
-}
-template <typename T, typename M> void Walk(std::list<T> &x, M &mutator) {
-  for (auto &elem : x) {
-    Walk(elem, mutator);
+  template <typename A, typename M>
+  static std::enable_if_t<!std::is_class_v<A> ||
+      std::is_same_v<std::string, A> || std::is_same_v<CharBlock, A>>
+  Walk(A &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      mutator.Post(x);
+    }
   }
-}
-template <typename V> void Walk(const Block &x, V &visitor) {
-  if (visitor.Pre(x)) {
+
+  // Traversal of needed STL template classes (optional, list, tuple, variant)
+  // For most lists, just traverse the elements; but when a list constitutes
+  // a Block (i.e., std::list<ExecutionPartConstruct>), also invoke the
+  // visitor/mutator on the list itself.
+  template <typename T, typename V>
+  static void Walk(const std::list<T> &x, V &visitor) {
     for (const auto &elem : x) {
       Walk(elem, visitor);
     }
-    visitor.Post(x);
   }
-}
-template <typename M> void Walk(Block &x, M &mutator) {
-  if (mutator.Pre(x)) {
+  template <typename T, typename M>
+  static void Walk(std::list<T> &x, M &mutator) {
     for (auto &elem : x) {
       Walk(elem, mutator);
     }
-    mutator.Post(x);
   }
-}
-template <typename T, typename V>
-void Walk(const std::optional<T> &x, V &visitor) {
-  if (x) {
-    Walk(*x, visitor);
+  template <typename V> static void Walk(const Block &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      for (const auto &elem : x) {
+        Walk(elem, visitor);
+      }
+      visitor.Post(x);
+    }
   }
-}
-template <typename T, typename M> void Walk(std::optional<T> &x, M &mutator) {
-  if (x) {
-    Walk(*x, mutator);
+  template <typename M> static void Walk(Block &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      for (auto &elem : x) {
+        Walk(elem, mutator);
+      }
+      mutator.Post(x);
+    }
   }
-}
-template <std::size_t I = 0, typename Func, typename T>
-void ForEachInTuple(const T &tuple, Func func) {
-  func(std::get<I>(tuple));
-  if constexpr (I + 1 < std::tuple_size_v<T>) {
-    ForEachInTuple<I + 1>(tuple, func);
+  template <typename T, typename V>
+  static void Walk(const std::optional<T> &x, V &visitor) {
+    if (x) {
+      Walk(*x, visitor);
+    }
   }
-}
-template <typename V, typename... A>
-void Walk(const std::tuple<A...> &x, V &visitor) {
-  if (sizeof...(A) > 0) {
+  template <typename T, typename M>
+  static void Walk(std::optional<T> &x, M &mutator) {
+    if (x) {
+      Walk(*x, mutator);
+    }
+  }
+  template <std::size_t I = 0, typename Func, typename T>
+  static void ForEachInTuple(const T &tuple, Func func) {
+    func(std::get<I>(tuple));
+    if constexpr (I + 1 < std::tuple_size_v<T>) {
+      ForEachInTuple<I + 1>(tuple, func);
+    }
+  }
+  template <typename V, typename... A>
+  static void Walk(const std::tuple<A...> &x, V &visitor) {
+    if (sizeof...(A) > 0) {
+      if (visitor.Pre(x)) {
+        ForEachInTuple(x, [&](const auto &y) { Walk(y, visitor); });
+        visitor.Post(x);
+      }
+    }
+  }
+  template <std::size_t I = 0, typename Func, typename T>
+  static void ForEachInTuple(T &tuple, Func func) {
+    func(std::get<I>(tuple));
+    if constexpr (I + 1 < std::tuple_size_v<T>) {
+      ForEachInTuple<I + 1>(tuple, func);
+    }
+  }
+  template <typename M, typename... A>
+  static void Walk(std::tuple<A...> &x, M &mutator) {
+    if (sizeof...(A) > 0) {
+      if (mutator.Pre(x)) {
+        ForEachInTuple(x, [&](auto &y) { Walk(y, mutator); });
+        mutator.Post(x);
+      }
+    }
+  }
+  template <typename V, typename... A>
+  static void Walk(const std::variant<A...> &x, V &visitor) {
     if (visitor.Pre(x)) {
-      ForEachInTuple(x, [&](const auto &y) { Walk(y, visitor); });
+      common::visit([&](const auto &y) { Walk(y, visitor); }, x);
       visitor.Post(x);
     }
   }
-}
-template <std::size_t I = 0, typename Func, typename T>
-void ForEachInTuple(T &tuple, Func func) {
-  func(std::get<I>(tuple));
-  if constexpr (I + 1 < std::tuple_size_v<T>) {
-    ForEachInTuple<I + 1>(tuple, func);
-  }
-}
-template <typename M, typename... A>
-void Walk(std::tuple<A...> &x, M &mutator) {
-  if (sizeof...(A) > 0) {
+  template <typename M, typename... A>
+  static void Walk(std::variant<A...> &x, M &mutator) {
     if (mutator.Pre(x)) {
-      ForEachInTuple(x, [&](auto &y) { Walk(y, mutator); });
+      common::visit([&](auto &y) { Walk(y, mutator); }, x);
       mutator.Post(x);
     }
   }
-}
-template <typename V, typename... A>
-void Walk(const std::variant<A...> &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    common::visit([&](const auto &y) { Walk(y, visitor); }, x);
-    visitor.Post(x);
+  template <typename A, typename B, typename V>
+  static void Walk(const std::pair<A, B> &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.first, visitor);
+      Walk(x.second, visitor);
+    }
   }
-}
-template <typename M, typename... A>
-void Walk(std::variant<A...> &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    common::visit([&](auto &y) { Walk(y, mutator); }, x);
-    mutator.Post(x);
+  template <typename A, typename B, typename M>
+  static void Walk(std::pair<A, B> &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.first, mutator);
+      Walk(x.second, mutator);
+    }
   }
-}
-template <typename A, typename B, typename V>
-void Walk(const std::pair<A, B> &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.first, visitor);
-    Walk(x.second, visitor);
+
+  // Trait-determined traversal of empty, tuple, union, wrapper,
+  // and constraint-checking classes.
+  template <typename A, typename V>
+  static std::enable_if_t<EmptyTrait<A>> Walk(const A &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      visitor.Post(x);
+    }
   }
-}
-template <typename A, typename B, typename M>
-void Walk(std::pair<A, B> &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.first, mutator);
-    Walk(x.second, mutator);
+  template <typename A, typename M>
+  static std::enable_if_t<EmptyTrait<A>> Walk(A &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      mutator.Post(x);
+    }
   }
-}
 
-// Trait-determined traversal of empty, tuple, union, wrapper,
-// and constraint-checking classes.
-template <typename A, typename V>
-std::enable_if_t<EmptyTrait<A>> Walk(const A &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    visitor.Post(x);
+  template <typename A, typename V>
+  static std::enable_if_t<TupleTrait<A>> Walk(const A &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.t, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename A, typename M>
-std::enable_if_t<EmptyTrait<A>> Walk(A &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    mutator.Post(x);
+  template <typename A, typename M>
+  static std::enable_if_t<TupleTrait<A>> Walk(A &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.t, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename A, typename V>
-std::enable_if_t<TupleTrait<A>> Walk(const A &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.t, visitor);
-    visitor.Post(x);
+  template <typename A, typename V>
+  static std::enable_if_t<UnionTrait<A>> Walk(const A &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.u, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename A, typename M>
-std::enable_if_t<TupleTrait<A>> Walk(A &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.t, mutator);
-    mutator.Post(x);
+  template <typename A, typename M>
+  static std::enable_if_t<UnionTrait<A>> Walk(A &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.u, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename A, typename V>
-std::enable_if_t<UnionTrait<A>> Walk(const A &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.u, visitor);
-    visitor.Post(x);
+  template <typename A, typename V>
+  static std::enable_if_t<WrapperTrait<A>> Walk(const A &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.v, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename A, typename M>
-std::enable_if_t<UnionTrait<A>> Walk(A &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.u, mutator);
-    mutator.Post(x);
+  template <typename A, typename M>
+  static std::enable_if_t<WrapperTrait<A>> Walk(A &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.v, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename A, typename V>
-std::enable_if_t<WrapperTrait<A>> Walk(const A &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.v, visitor);
-    visitor.Post(x);
+  template <typename A, typename V>
+  static std::enable_if_t<ConstraintTrait<A>> Walk(const A &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.thing, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename A, typename M>
-std::enable_if_t<WrapperTrait<A>> Walk(A &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.v, mutator);
-    mutator.Post(x);
+  template <typename A, typename M>
+  static std::enable_if_t<ConstraintTrait<A>> Walk(A &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.thing, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename A, typename V>
-std::enable_if_t<ConstraintTrait<A>> Walk(const A &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.thing, visitor);
-    visitor.Post(x);
+  template <typename T, typename V>
+  static void Walk(const common::Indirection<T> &x, V &visitor) {
+    Walk(x.value(), visitor);
   }
-}
-template <typename A, typename M>
-std::enable_if_t<ConstraintTrait<A>> Walk(A &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.thing, mutator);
-    mutator.Post(x);
+  template <typename T, typename M>
+  static void Walk(common::Indirection<T> &x, M &mutator) {
+    Walk(x.value(), mutator);
   }
-}
-
-template <typename T, typename V>
-void Walk(const common::Indirection<T> &x, V &visitor) {
-  Walk(x.value(), visitor);
-}
-template <typename T, typename M>
-void Walk(common::Indirection<T> &x, M &mutator) {
-  Walk(x.value(), mutator);
-}
 
-template <typename T, typename V> void Walk(const Statement<T> &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    // N.B. The label, if any, is not visited.
-    Walk(x.source, visitor);
-    Walk(x.statement, visitor);
-    visitor.Post(x);
+  template <typename T, typename V>
+  static void Walk(const Statement<T> &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      // N.B. The label, if any, is not visited.
+      Walk(x.source, visitor);
+      Walk(x.statement, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename T, typename M> void Walk(Statement<T> &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    // N.B. The label, if any, is not visited.
-    Walk(x.source, mutator);
-    Walk(x.statement, mutator);
-    mutator.Post(x);
+  template <typename T, typename M>
+  static void Walk(Statement<T> &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      // N.B. The label, if any, is not visited.
+      Walk(x.source, mutator);
+      Walk(x.statement, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename T, typename V>
-void Walk(const UnlabeledStatement<T> &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.source, visitor);
-    Walk(x.statement, visitor);
-    visitor.Post(x);
+  template <typename T, typename V>
+  static void Walk(const UnlabeledStatement<T> &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.source, visitor);
+      Walk(x.statement, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename T, typename M>
-void Walk(UnlabeledStatement<T> &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.source, mutator);
-    Walk(x.statement, mutator);
-    mutator.Post(x);
+  template <typename T, typename M>
+  static void Walk(UnlabeledStatement<T> &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.source, mutator);
+      Walk(x.statement, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename V> void Walk(const Name &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.source, visitor);
-    visitor.Post(x);
+  template <typename V> static void Walk(const Name &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.source, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(Name &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.source, mutator);
-    mutator.Post(x);
+  template <typename M> static void Walk(Name &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.source, mutator);
+      mutator.Post(x);
+    }
   }
-}
 
-template <typename V> void Walk(const AcSpec &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.type, visitor);
-    Walk(x.values, visitor);
-    visitor.Post(x);
+  template <typename V> static void Walk(const AcSpec &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.type, visitor);
+      Walk(x.values, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(AcSpec &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.type, mutator);
-    Walk(x.values, mutator);
-    mutator.Post(x);
+  template <typename M> static void Walk(AcSpec &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.type, mutator);
+      Walk(x.values, mutator);
+      mutator.Post(x);
+    }
   }
-}
-template <typename V> void Walk(const ArrayElement &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.base, visitor);
-    Walk(x.subscripts, visitor);
-    visitor.Post(x);
+  template <typename V> static void Walk(const ArrayElement &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.base, visitor);
+      Walk(x.subscripts, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(ArrayElement &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.base, mutator);
-    Walk(x.subscripts, mutator);
-    mutator.Post(x);
+  template <typename M> static void Walk(ArrayElement &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.base, mutator);
+      Walk(x.subscripts, mutator);
+      mutator.Post(x);
+    }
   }
-}
-template <typename V>
-void Walk(const CharSelector::LengthAndKind &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.length, visitor);
-    Walk(x.kind, visitor);
-    visitor.Post(x);
+  template <typename V>
+  static void Walk(const CharSelector::LengthAndKind &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.length, visitor);
+      Walk(x.kind, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(CharSelector::LengthAndKind &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.length, mutator);
-    Walk(x.kind, mutator);
-    mutator.Post(x);
+  template <typename M>
+  static void Walk(CharSelector::LengthAndKind &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.length, mutator);
+      Walk(x.kind, mutator);
+      mutator.Post(x);
+    }
   }
-}
-template <typename V> void Walk(const CaseValueRange::Range &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.lower, visitor);
-    Walk(x.upper, visitor);
-    visitor.Post(x);
+  template <typename V>
+  static void Walk(const CaseValueRange::Range &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.lower, visitor);
+      Walk(x.upper, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(CaseValueRange::Range &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.lower, mutator);
-    Walk(x.upper, mutator);
-    mutator.Post(x);
+  template <typename M> static void Walk(CaseValueRange::Range &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.lower, mutator);
+      Walk(x.upper, mutator);
+      mutator.Post(x);
+    }
   }
-}
-template <typename V> void Walk(const CoindexedNamedObject &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.base, visitor);
-    Walk(x.imageSelector, visitor);
-    visitor.Post(x);
+  template <typename V>
+  static void Walk(const CoindexedNamedObject &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.base, visitor);
+      Walk(x.imageSelector, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(CoindexedNamedObject &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.base, mutator);
-    Walk(x.imageSelector, mutator);
-    mutator.Post(x);
+  template <typename M> static void Walk(CoindexedNamedObject &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.base, mutator);
+      Walk(x.imageSelector, mutator);
+      mutator.Post(x);
+    }
   }
-}
-template <typename V>
-void Walk(const DeclarationTypeSpec::Class &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.derived, visitor);
-    visitor.Post(x);
+  template <typename V>
+  static void Walk(const DeclarationTypeSpec::Class &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.derived, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(DeclarationTypeSpec::Class &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.derived, mutator);
-    mutator.Post(x);
+  template <typename M>
+  static void Walk(DeclarationTypeSpec::Class &x, M &mutator) {
+    if (mutator.Pre(x)) {
+      Walk(x.derived, mutator);
+      mutator.Post(x);
+    }
   }
-}
-template <typename V>
-void Walk(const DeclarationTypeSpec::Type &x, V &visitor) {
-  if (visitor.Pre(x)) {
-    Walk(x.derived, visitor);
-    visitor.Post(x);
+  template <typename V>
+  static void Walk(const DeclarationTypeSpec::Type &x, V &visitor) {
+    if (visitor.Pre(x)) {
+      Walk(x.derived, visitor);
+      visitor.Post(x);
+    }
   }
-}
-template <typename M> void Walk(DeclarationTypeSpec::Type &x, M &mutator) {
-  if (mutator.Pre(x)) {
-    Walk(x.derived, mutator);
-    mutator.Post(x);
+  template <type...
[truncated]

Copy link
Contributor

@klausler klausler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is broken that this patch fixes?

@kparzysz
Copy link
Contributor Author

This change is

  • wrapping all the definitions into a class detail::SomeUnlikelyName,
  • adding two global-namespace::Walk functions to call the static members,
  • clang-formatting the whole file

@kparzysz
Copy link
Contributor Author

kparzysz commented Nov 12, 2024

what is broken that this patch fixes?
I'm trying to add more nodes for OpenMP, and when I have a variant inside of a tuple, the visitor breaks.

How does it break? Can it be fixed with a smaller change?

@kparzysz
Copy link
Contributor Author

what is broken that this patch fixes?
I'm trying to add more nodes for OpenMP, and when I have a variant inside of a tuple, the visitor breaks.

How does it break? Can it be fixed with a smaller change?

It fails to compile, because the call to Walk from ForEachInTuple can't use the UnionTrait overload.

It could be remedied with just adding a declaration for the variant overload, but the actual fix would be either using a class, or declaring all the overloads ahead of the definitions.

@klausler
Copy link
Contributor

It could be remedied with just adding a declaration for the variant overload, but the actual fix would be either using a class, or declaring all the overloads ahead of the definitions.

This is unclear to me. Why would a declaration for the std::variant overload not solve the problem?

@kparzysz
Copy link
Contributor Author

This is unclear to me. Why would a declaration for the std::variant overload not solve the problem?

It would solve the problem I'm having, but it wouldn't address any latent problems of the same kind that may arise later. The two approaches I listed guarantee that any two Walk functions can call each other.

@kparzysz kparzysz merged commit deb057a into llvm:main Nov 13, 2024
11 checks passed
@kparzysz kparzysz deleted the users/kparzysz/flang-parse-tree-visitor branch November 13, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:parser flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants