[NFC] Clarify and standardize order in flexibleCopy (#6749)

flexibleCopy always visited parents before children, but it visited
vector children in reverse order:

(call        ;; 1
  (call $a)  ;; 3
  (call $b)  ;; 2
)

The order of children happened to not matter in any user of this code,
and that's just what you get when you iterate over children in a vector
and push them to a stack before visiting them, so this odd ordering
was not noticed.

For a new user I will introduce soon, however, it would be nice to have
the normal pre-order:

(call        ;; 1
  (call $a)  ;; 2
  (call $b)  ;; 3
)
(2 & 3 swapped).

This cannot be tested in the current code as it is NFC, but the later PR
will depend on it and test it heavily.

2 files changed, 13 insertions, 1 deletions

diff --git a/src/ir/ExpressionManipulator.cpp b/src/ir/ExpressionManipulator.cpp
index 4cac87a01..51ed7552d 100644
--- a/src/ir/ExpressionManipulator.cpp
+++ b/src/ir/ExpressionManipulator.cpp
@@ -62,9 +62,10 @@ flexibleCopy(Expression* original, Module& wasm, CustomCopier custom) {
 #define DELEGATE_FIELD_CHILD(id, field)                                        \
   tasks.push_back({castOriginal->field, &castCopy->field});
 
+// Iterate in reverse order here so we visit children in normal order.
 #define DELEGATE_FIELD_CHILD_VECTOR(id, field)                                 \
   castCopy->field.resize(castOriginal->field.size());                          \
-  for (Index i = 0; i < castOriginal->field.size(); i++) {                     \
+  for (auto i = int64_t(castOriginal->field.size()) - 1; i >= 0; i--) {        \
     tasks.push_back({castOriginal->field[i], &castCopy->field[i]});            \
   }
 
diff --git a/src/ir/manipulation.h b/src/ir/manipulation.h
index 64cd15dc3..e7816af9f 100644
--- a/src/ir/manipulation.h
+++ b/src/ir/manipulation.h
@@ -68,6 +68,17 @@ inline OutputType* convert(InputType* input, MixedArena& allocator) {
 // expression before copying it. If it returns a non-null value then that is
 // used (effectively overriding the normal copy), and if it is null then we do a
 // normal copy.
+//
+// The order of iteration here is *pre*-order, that is, parents before children,
+// so that it is possible to override an expression and all its children.
+// Children themselves are visited in normal order. For example, this is the
+// order of the following expression:
+//
+//  (i32.add     ;; visited first (and children not visited, if overridden)
+//    (call $a)  ;; visited second
+//    (call $b)  ;; visited third
+//  )
+//
 using CustomCopier = std::function<Expression*(Expression*)>;
 Expression*
 flexibleCopy(Expression* original, Module& wasm, CustomCopier custom);