Fix type conflicts preventing text round-tripping

src/wasm-type.h

@@ -17,6 +17,7 @@
 #ifndef wasm_wasm_type_h
 #define wasm_wasm_type_h
 
+#include <algorithm>
 #include <functional>
 #include <optional>
 #include <ostream>
@@ -591,6 +592,23 @@ class Type {
 };
 
 Type Type::asWrittenGivenFeatures(FeatureSet feats) const {
+  if (isTuple()) {
+    // Check whether we would change anything before doing the work of
+    // constructing a new tuple type.
+    const auto& tuple = getTuple();
+    bool hasChange = std::any_of(tuple.begin(), tuple.end(), [&](Type t) {
+      return t.asWrittenGivenFeatures(feats) != t;
+    });
+    if (!hasChange) {
+      return *this;
+    }
+    std::vector<Type> elems;
+    elems.reserve(size());
+    for (Index i = 0; i < size(); ++i) {
+      elems.push_back(tuple[i].asWrittenGivenFeatures(feats));
+    }
+    return Type(elems);
+  }
   if (!isRef()) {
     return *this;
   }

src/wasm/wasm-ir-builder.cpp

@@ -76,15 +76,22 @@ MaybeResult<IRBuilder::HoistedVal> IRBuilder::hoistLastValue() {
     return HoistedVal{Index(index), nullptr};
   }
   auto*& expr = stack[index];
-  auto type = expr->type;
-  if (type == Type::unreachable) {
+  if (expr->type == Type::unreachable) {
     // Make sure the top of the stack also has an unreachable expression.
     if (stack.back()->type != Type::unreachable) {
       pushSynthetic(builder.makeUnreachable());
     }
     return HoistedVal{Index(index), nullptr};
   }
-  // Hoist with a scratch local.
+  // Hoist with a scratch local. Normally the scratch local is the same type as
+  // the hoisted expression, but we may need to adjust it given the enabled
+  // features. Otherwise, if the expression has a tuple type with a more refined
+  // element than would be written to a binary, then that refined element type
+  // would end up in a multivalue block return. But that could cause us to fail
+  // text roundtripping if the block type would conflict after binary writing
+  // with another function type in the module. Avoid this problem by
+  // generalizing the scratch local type eagerly.
+  auto type = expr->type.asWrittenGivenFeatures(wasm.features);
   auto scratchIdx = addScratchLocal(type);
   CHECK_ERR(scratchIdx);
   expr = builder.makeLocalSet(*scratchIdx, expr);

test/lit/basic/scratch-local-roundtrip.wast

@@ -0,0 +1,41 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
+;; RUN: wasm-opt %s -all --disable-gc -S -o - | filecheck %s
+;; RUN: wasm-opt %s -all --disable-gc -S -o - | wasm-opt -all --disable-gc -S -o - | filecheck %s
+
+(module
+  ;; RTRIP:      (type $tuple (func (result i32 externref)))
+  ;; CHECK:      (type $tuple (func (result i32 externref)))
+  (type $tuple (func (result i32 externref)))
+
+  ;; CHECK:      (func $f (result i32 externref)
+  ;; CHECK-NEXT:  (local $scratch (tuple i32 externref))
+  ;; CHECK-NEXT:  (block $l (type $tuple) (result i32 externref)
+  ;; CHECK-NEXT:   (br $l
+  ;; CHECK-NEXT:    (block (type $tuple) (result i32 externref)
+  ;; CHECK-NEXT:     (local.set $scratch
+  ;; CHECK-NEXT:      (br_if $l
+  ;; CHECK-NEXT:       (tuple.make 2
+  ;; CHECK-NEXT:        (i32.const 0)
+  ;; CHECK-NEXT:        (ref.null noextern)
+  ;; CHECK-NEXT:       )
+  ;; CHECK-NEXT:       (i32.const 0)
+  ;; CHECK-NEXT:      )
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:     (nop)
+  ;; CHECK-NEXT:     (local.get $scratch)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $f (result i32 externref)
+    block $l (type $tuple) (result i32 externref)
+      i32.const 0
+      ref.null extern
+      i32.const 0
+      br_if $l ;; package the i32 and nullexternref into a tuple
+      nop      ;; force creation of a scratch local
+      br $l    ;; consume a scratch local block with (result i32 nullexternref)
+               ;; the nullexternref will become externref, as gc is disabled.
+    end
+  )
+)