Unify method pairs with and without Type param (#6184)

As suggested in
#6181 (comment),
using `std::optional<Type>`, this unifies two different versions of
`make***`, for  block-like structures (`block`, `if`, `loop`, `try`, and
`try_table`) with and without a type parameter.

This also allows unifying of `finalize` methods, with and without a
type. This also sets `breakability` argument of `Block::finalize` to
`Unknown` so we can only have one `Block::finalize` that handles all
cases.

This also adds an optional `std::optional<Type> type` parameter to
`blockifyWithName`, and `makeSequence` functions in `wasm-builder.h`.
blockify was not included because it has a variadic parameter.

Author: aheejin

src/wasm-builder.h

@@ -20,6 +20,7 @@
 #include "ir/manipulation.h"
 #include "parsing.h"
 #include "wasm.h"
+#include <optional>
 
 namespace wasm {
 
@@ -189,69 +190,47 @@ class Builder {
                      bool>;
 
   template<typename T, bool_if_not_expr_t<T> = true>
-  Block* makeBlock(const T& items) {
-    auto* ret = wasm.allocator.alloc<Block>();
-    ret->list.set(items);
-    ret->finalize();
-    return ret;
-  }
-
-  template<typename T, bool_if_not_expr_t<T> = true>
-  Block* makeBlock(const T& items, Type type) {
+  Block* makeBlock(const T& items, std::optional<Type> type = std::nullopt) {
     auto* ret = wasm.allocator.alloc<Block>();
     ret->list.set(items);
     ret->finalize(type);
     return ret;
   }
 
   template<typename T, bool_if_not_expr_t<T> = true>
-  Block* makeBlock(Name name, const T& items, Type type) {
+  Block* makeBlock(Name name,
+                   const T& items,
+                   std::optional<Type> type = std::nullopt) {
     auto* ret = wasm.allocator.alloc<Block>();
     ret->name = name;
     ret->list.set(items);
     ret->finalize(type);
     return ret;
   }
-  Block* makeBlock(std::initializer_list<Expression*>&& items) {
-    return makeBlock(items);
-  }
-  Block* makeBlock(std::initializer_list<Expression*>&& items, Type type) {
+  Block* makeBlock(std::initializer_list<Expression*>&& items,
+                   std::optional<Type> type = std::nullopt) {
     return makeBlock(items, type);
   }
-  Block*
-  makeBlock(Name name, std::initializer_list<Expression*>&& items, Type type) {
+  Block* makeBlock(Name name,
+                   std::initializer_list<Expression*>&& items,
+                   std::optional<Type> type = std::nullopt) {
     return makeBlock(name, items, type);
   }
 
   If* makeIf(Expression* condition,
              Expression* ifTrue,
-             Expression* ifFalse = nullptr) {
-    auto* ret = wasm.allocator.alloc<If>();
-    ret->condition = condition;
-    ret->ifTrue = ifTrue;
-    ret->ifFalse = ifFalse;
-    ret->finalize();
-    return ret;
-  }
-  If* makeIf(Expression* condition,
-             Expression* ifTrue,
-             Expression* ifFalse,
-             Type type) {
+             Expression* ifFalse = nullptr,
+             std::optional<Type> type = std::nullopt) {
     auto* ret = wasm.allocator.alloc<If>();
     ret->condition = condition;
     ret->ifTrue = ifTrue;
     ret->ifFalse = ifFalse;
     ret->finalize(type);
     return ret;
   }
-  Loop* makeLoop(Name name, Expression* body) {
-    auto* ret = wasm.allocator.alloc<Loop>();
-    ret->name = name;
-    ret->body = body;
-    ret->finalize();
-    return ret;
-  }
-  Loop* makeLoop(Name name, Expression* body, Type type) {
+  Loop* makeLoop(Name name,
+                 Expression* body,
+                 std::optional<Type> type = std::nullopt) {
     auto* ret = wasm.allocator.alloc<Loop>();
     ret->name = name;
     ret->body = body;
@@ -792,77 +771,47 @@ class Builder {
                const std::vector<Name>& catchTags,
                const std::vector<Expression*>& catchBodies,
                Name delegateTarget,
-               Type type,
-               bool hasType) { // differentiate whether a type was passed in
+               std::optional<Type> type = std::nullopt) {
     auto* ret = wasm.allocator.alloc<Try>();
     ret->name = name;
     ret->body = body;
     ret->catchTags.set(catchTags);
     ret->catchBodies.set(catchBodies);
-    if (hasType) {
-      ret->finalize(type);
-    } else {
-      ret->finalize();
-    }
+    ret->finalize(type);
     return ret;
   }
 
 public:
-  Try* makeTry(Expression* body,
-               const std::vector<Name>& catchTags,
-               const std::vector<Expression*>& catchBodies) {
-    return makeTry(
-      Name(), body, catchTags, catchBodies, Name(), Type::none, false);
-  }
+  // TODO delete?
   Try* makeTry(Expression* body,
                const std::vector<Name>& catchTags,
                const std::vector<Expression*>& catchBodies,
-               Type type) {
-    return makeTry(Name(), body, catchTags, catchBodies, Name(), type, true);
-  }
-  Try* makeTry(Name name,
-               Expression* body,
-               const std::vector<Name>& catchTags,
-               const std::vector<Expression*>& catchBodies) {
-    return makeTry(
-      name, body, catchTags, catchBodies, Name(), Type::none, false);
+               std::optional<Type> type = std::nullopt) {
+    return makeTry(Name(), body, catchTags, catchBodies, Name(), type);
   }
   Try* makeTry(Name name,
                Expression* body,
                const std::vector<Name>& catchTags,
                const std::vector<Expression*>& catchBodies,
-               Type type) {
-    return makeTry(name, body, catchTags, catchBodies, Name(), type, true);
-  }
-  Try* makeTry(Expression* body, Name delegateTarget) {
-    return makeTry(Name(), body, {}, {}, delegateTarget, Type::none, false);
-  }
-  Try* makeTry(Expression* body, Name delegateTarget, Type type) {
-    return makeTry(Name(), body, {}, {}, delegateTarget, type, true);
+               std::optional<Type> type = std::nullopt) {
+    return makeTry(name, body, catchTags, catchBodies, Name(), type);
   }
-  Try* makeTry(Name name, Expression* body, Name delegateTarget) {
-    return makeTry(name, body, {}, {}, delegateTarget, Type::none, false);
-  }
-  Try* makeTry(Name name, Expression* body, Name delegateTarget, Type type) {
-    return makeTry(name, body, {}, {}, delegateTarget, type, true);
+  Try* makeTry(Expression* body,
+               Name delegateTarget,
+               std::optional<Type> type = std::nullopt) {
+    return makeTry(Name(), body, {}, {}, delegateTarget, type);
   }
-  TryTable* makeTryTable(Expression* body,
-                         const std::vector<Name>& catchTags,
-                         const std::vector<Name>& catchDests,
-                         const std::vector<bool>& catchRefs) {
-    auto* ret = wasm.allocator.alloc<TryTable>();
-    ret->body = body;
-    ret->catchTags.set(catchTags);
-    ret->catchDests.set(catchDests);
-    ret->catchRefs.set(catchRefs);
-    ret->finalize(&wasm);
-    return ret;
+  Try* makeTry(Name name,
+               Expression* body,
+               Name delegateTarget,
+               std::optional<Type> type = std::nullopt) {
+    return makeTry(name, body, {}, {}, delegateTarget, type);
   }
   TryTable* makeTryTable(Expression* body,
                          const std::vector<Name>& catchTags,
                          const std::vector<Name>& catchDests,
                          const std::vector<bool>& catchRefs,
-                         Type type) {
+                         std::optional<Type> type = std::nullopt) {
     auto* ret = wasm.allocator.alloc<TryTable>();
     ret->body = body;
     ret->catchTags.set(catchTags);
@@ -1359,8 +1308,10 @@ class Builder {
   // ensure a node is a block, if it isn't already, and optionally append to the
   // block this variant sets a name for the block, so it will not reuse a block
   // already named
-  Block*
-  blockifyWithName(Expression* any, Name name, Expression* append = nullptr) {
+  Block* blockifyWithName(Expression* any,
+                          Name name,
+                          Expression* append = nullptr,
+                          std::optional<Type> type = std::nullopt) {
     Block* block = nullptr;
     if (any) {
       block = any->dynCast<Block>();
@@ -1371,21 +1322,16 @@ class Builder {
     block->name = name;
     if (append) {
       block->list.push_back(append);
-      block->finalize();
+      block->finalize(type);
     }
     return block;
   }
 
   // a helper for the common pattern of a sequence of two expressions. Similar
   // to blockify, but does *not* reuse a block if the first is one.
-  Block* makeSequence(Expression* left, Expression* right) {
-    auto* block = makeBlock(left);
-    block->list.push_back(right);
-    block->finalize();
-    return block;
-  }
-
-  Block* makeSequence(Expression* left, Expression* right, Type type) {
+  Block* makeSequence(Expression* left,
+                      Expression* right,
+                      std::optional<Type> type = std::nullopt) {
     auto* block = makeBlock(left);
     block->list.push_back(right);
     block->finalize(type);

src/wasm.h

@@ -828,23 +828,21 @@ class Block : public SpecificExpression<Expression::BlockId> {
   Name name;
   ExpressionList list;
 
-  // set the type purely based on its contents. this scans the block, so it is
-  // not fast.
-  void finalize();
-
-  // set the type given you know its type, which is the case when parsing
-  // s-expression or binary, as explicit types are given. the only additional
-  // work this does is to set the type to unreachable in the cases that is
-  // needed (which may require scanning the block)
-  void finalize(Type type_);
-
   enum Breakability { Unknown, HasBreak, NoBreak };
 
-  // set the type given you know its type, and you know if there is a break to
-  // this block. this avoids the need to scan the contents of the block in the
-  // case that it might be unreachable, so it is recommended if you already know
-  // the type and breakability anyhow.
-  void finalize(Type type_, Breakability breakability);
+  // If type_ is not given, set the type purely based on its contents. this
+  // scans the block, so it is not fast.
+  // If type_ is given, set the type given you know its type, which is the case
+  // when parsing s-expression or binary, as explicit types are given. the only
+  // additional work this does is to set the type to unreachable in the cases
+  // that is needed (which may require scanning the block)
+  //
+  // If breakability is given, you know if there is a break to this block. this
+  // avoids the need to scan the contents of the block in the case that it might
+  // be unreachable, so it is recommended if you already know the type and
+  // breakability anyhow.
+  void finalize(std::optional<Type> type_ = std::nullopt,
+                Breakability breakability = Unknown);
 };
 
 class If : public SpecificExpression<Expression::IfId> {
@@ -856,14 +854,12 @@ class If : public SpecificExpression<Expression::IfId> {
   Expression* ifTrue;
   Expression* ifFalse;
 
-  // set the type given you know its type, which is the case when parsing
-  // s-expression or binary, as explicit types are given. the only additional
-  // work this does is to set the type to unreachable in the cases that is
-  // needed.
-  void finalize(Type type_);
-
-  // set the type purely based on its contents.
-  void finalize();
+  // If type_ is not given, set the type purely based on its contents.
+  // If type_ is given, set the type given you know its type, which is the case
+  // when parsing s-expression or binary, as explicit types are given. the only
+  // additional work this does is to set the type to unreachable in the cases
+  // that is needed.
+  void finalize(std::optional<Type> type_ = std::nullopt);
 };
 
 class Loop : public SpecificExpression<Expression::LoopId> {
@@ -874,14 +870,12 @@ class Loop : public SpecificExpression<Expression::LoopId> {
   Name name;
   Expression* body;
 
-  // set the type given you know its type, which is the case when parsing
-  // s-expression or binary, as explicit types are given. the only additional
-  // work this does is to set the type to unreachable in the cases that is
-  // needed.
-  void finalize(Type type_);
-
-  // set the type purely based on its contents.
-  void finalize();
+  // If type_ is not given, set the type purely based on its contents.
+  // If type_ is given, set the type given you know its type, which is the case
+  // when parsing s-expression or binary, as explicit types are given. the only
+  // additional work this does is to set the type to unreachable in the cases
+  // that is needed.
+  void finalize(std::optional<Type> type_ = std::nullopt);
 };
 
 class Break : public SpecificExpression<Expression::BreakId> {
@@ -1472,8 +1466,7 @@ class Try : public SpecificExpression<Expression::TryId> {
   }
   bool isCatch() const { return !catchBodies.empty(); }
   bool isDelegate() const { return delegateTarget.is(); }
-  void finalize();
-  void finalize(Type type_);
+  void finalize(std::optional<Type> type_ = std::nullopt);
 };
 
 // 'try_table' from the new EH proposal
@@ -1497,8 +1490,8 @@ class TryTable : public SpecificExpression<Expression::TryTableId> {
   // When 'Module*' parameter is given, we cache catch tags' types into
   // 'sentTypes' array, so that the types can be accessed in other analyses
   // without accessing the module.
-  void finalize(Module* wasm = nullptr);
-  void finalize(Type type_, Module* wasm = nullptr);
+  void finalize(std::optional<Type> type_ = std::nullopt,
+                Module* wasm = nullptr);
 
   // Caches tags' types in the catch clauses in order not to query the module
   // every time we query the sent types