tint/transform: Add HoistToDeclBefore::VariableKind

Enums are scientifically proven to be 96.4% better than a bool for parameters.

Also throw in kConst because we can.

Change-Id: I788504d8d452d6a879d2d675891e3171db6a40f2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/111244
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>

src/tint/transform/promote_initializers_to_let.cc

@@ -70,7 +70,8 @@ Transform::ApplyResult PromoteInitializersToLet::Apply(const Program* src,
         }
 
         any_promoted = true;
-        return hoist_to_decl_before.Add(expr, expr->Declaration(), true);
+        return hoist_to_decl_before.Add(expr, expr->Declaration(),
+                                        HoistToDeclBefore::VariableKind::kLet);
     };
 
     for (auto* node : src->ASTNodes().Objects()) {

src/tint/transform/utils/hoist_to_decl_before.cc

@@ -38,23 +38,39 @@ struct HoistToDeclBefore::State {
     /// @copydoc HoistToDeclBefore::Add()
     bool Add(const sem::Expression* before_expr,
              const ast::Expression* expr,
-             bool as_let,
+             VariableKind kind,
              const char* decl_name) {
         auto name = b.Symbols().New(decl_name);
 
-        if (as_let) {
+        switch (kind) {
-            auto builder = [this, expr, name] {
+            case VariableKind::kLet: {
-                return b.Decl(b.Let(name, ctx.CloneWithoutTransform(expr)));
+                auto builder = [this, expr, name] {
-            };
+                    return b.Decl(b.Let(name, ctx.CloneWithoutTransform(expr)));
-            if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
+                };
-                return false;
+                if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
+                    return false;
+                }
+                break;
             }
-        } else {
+
-            auto builder = [this, expr, name] {
+            case VariableKind::kVar: {
-                return b.Decl(b.Var(name, ctx.CloneWithoutTransform(expr)));
+                auto builder = [this, expr, name] {
-            };
+                    return b.Decl(b.Var(name, ctx.CloneWithoutTransform(expr)));
-            if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
+                };
-                return false;
+                if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
+                    return false;
+                }
+                break;
+            }
+
+            case VariableKind::kConst: {
+                auto builder = [this, expr, name] {
+                    return b.Decl(b.Const(name, ctx.CloneWithoutTransform(expr)));
+                };
+                if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
+                    return false;
+                }
+                break;
             }
         }
 
@@ -361,9 +377,9 @@ HoistToDeclBefore::~HoistToDeclBefore() {}
 
 bool HoistToDeclBefore::Add(const sem::Expression* before_expr,
                             const ast::Expression* expr,
-                            bool as_let,
+                            VariableKind kind,
                             const char* decl_name) {
-    return state_->Add(before_expr, expr, as_let, decl_name);
+    return state_->Add(before_expr, expr, kind, decl_name);
 }
 
 bool HoistToDeclBefore::InsertBefore(const sem::Statement* before_stmt,

src/tint/transform/utils/hoist_to_decl_before.h

@@ -38,16 +38,23 @@ class HoistToDeclBefore {
     /// StmtBuilder is a builder of an AST statement
     using StmtBuilder = std::function<const ast::Statement*()>;
 
+    /// VariableKind is either a var, let or const
+    enum class VariableKind {
+        kVar,
+        kLet,
+        kConst,
+    };
+
     /// Hoists @p expr to a `let` or `var` with optional `decl_name`, inserting it
     /// before @p before_expr.
     /// @param before_expr expression to insert `expr` before
     /// @param expr expression to hoist
-    /// @param as_let hoist to `let` if true, otherwise to `var`
+    /// @param kind variable kind to hoist to
     /// @param decl_name optional name to use for the variable/constant name
     /// @return true on success
     bool Add(const sem::Expression* before_expr,
              const ast::Expression* expr,
-             bool as_let,
+             VariableKind kind,
              const char* decl_name = "");
 
     /// Inserts @p stmt before @p before_stmt, possibly converting 'for-loop's to 'loop's if

src/tint/transform/utils/hoist_to_decl_before_test.cc

@@ -44,7 +44,7 @@ TEST_F(HoistToDeclBeforeTest, VarInit) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kLet);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
@@ -75,14 +75,14 @@ TEST_F(HoistToDeclBeforeTest, ForLoopInit) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kVar);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
 
     auto* expect = R"(
 fn f() {
-  let tint_symbol = 1i;
+  var tint_symbol = 1i;
   for(var a = tint_symbol; true; ) {
   }
 }
@@ -93,12 +93,12 @@ fn f() {
 
 TEST_F(HoistToDeclBeforeTest, ForLoopCond) {
     // fn f() {
-    //     var a : bool;
+    //     const a = true;
     //     for(; a; ) {
     //     }
     // }
     ProgramBuilder b;
-    auto* var = b.Decl(b.Var("a", b.ty.bool_()));
+    auto* var = b.Decl(b.Const("a", b.Expr(true)));
     auto* expr = b.Expr("a");
     auto* s = b.For(nullptr, expr, nullptr, b.Block());
     b.Func("f", utils::Empty, b.ty.void_(), utils::Vector{var, s});
@@ -109,16 +109,16 @@ TEST_F(HoistToDeclBeforeTest, ForLoopCond) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kConst);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
 
     auto* expect = R"(
 fn f() {
-  var a : bool;
+  const a = true;
   loop {
-    let tint_symbol = a;
+    const tint_symbol = a;
     if (!(tint_symbol)) {
       break;
     }
@@ -147,7 +147,7 @@ TEST_F(HoistToDeclBeforeTest, ForLoopCont) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kLet);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
@@ -190,7 +190,7 @@ TEST_F(HoistToDeclBeforeTest, WhileCond) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kVar);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
@@ -199,7 +199,7 @@ TEST_F(HoistToDeclBeforeTest, WhileCond) {
 fn f() {
   var a : bool;
   loop {
-    let tint_symbol = a;
+    var tint_symbol = a;
     if (!(tint_symbol)) {
       break;
     }
@@ -214,14 +214,14 @@ fn f() {
 
 TEST_F(HoistToDeclBeforeTest, ElseIf) {
     // fn f() {
-    //     var a : bool;
+    //     const a = true;
     //     if (true) {
     //     } else if (a) {
     //     } else {
     //     }
     // }
     ProgramBuilder b;
-    auto* var = b.Decl(b.Var("a", b.ty.bool_()));
+    auto* var = b.Decl(b.Const("a", b.Expr(true)));
     auto* expr = b.Expr("a");
     auto* s = b.If(b.Expr(true), b.Block(),      //
                    b.Else(b.If(expr, b.Block(),  //
@@ -234,17 +234,17 @@ TEST_F(HoistToDeclBeforeTest, ElseIf) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kConst);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
 
     auto* expect = R"(
 fn f() {
-  var a : bool;
+  const a = true;
   if (true) {
   } else {
-    let tint_symbol = a;
+    const tint_symbol = a;
     if (tint_symbol) {
     } else {
     }
@@ -272,7 +272,7 @@ TEST_F(HoistToDeclBeforeTest, Array1D) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kLet);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
@@ -306,7 +306,7 @@ TEST_F(HoistToDeclBeforeTest, Array2D) {
 
     HoistToDeclBefore hoistToDeclBefore(ctx);
     auto* sem_expr = ctx.src->Sem().Get(expr);
-    hoistToDeclBefore.Add(sem_expr, expr, true);
+    hoistToDeclBefore.Add(sem_expr, expr, HoistToDeclBefore::VariableKind::kVar);
 
     ctx.Clone();
     Program cloned(std::move(cloned_b));
@@ -314,7 +314,7 @@ TEST_F(HoistToDeclBeforeTest, Array2D) {
     auto* expect = R"(
 fn f() {
   var a : array<array<i32, 10u>, 10i>;
-  let tint_symbol = a[0i][0i];
+  var tint_symbol = a[0i][0i];
   var b = tint_symbol;
 }
 )";

src/tint/transform/var_for_dynamic_index.cc

@@ -54,7 +54,8 @@ Transform::ApplyResult VarForDynamicIndex::Apply(const Program* src,
 
         // TODO(bclayton): group multiple accesses in the same object.
         // e.g. arr[i] + arr[i+1] // Don't create two vars for this
-        return hoist_to_decl_before.Add(indexed, object_expr, false, "var_for_index");
+        return hoist_to_decl_before.Add(indexed, object_expr, HoistToDeclBefore::VariableKind::kVar,
+                                        "var_for_index");
     };
 
     bool index_accessor_found = false;