review comments

Author: kripken

src/passes/MergeBlocks.cpp

@@ -73,7 +73,7 @@ namespace wasm {
 // For example, if there is a switch targeting us, we can't do it - we can't remove the value from other targets
 struct ProblemFinder : public ControlFlowWalker<ProblemFinder> {
   Name origin;
-  bool noWay = false;
+  bool foundProblem = false;
   // count br_ifs, and dropped br_ifs. if they don't match, then a br_if flow value is used, and we can't drop it
   Index brIfs = 0;
   Index droppedBrIfs = 0;
@@ -88,7 +88,7 @@ struct ProblemFinder : public ControlFlowWalker<ProblemFinder> {
       }
       // if the value has side effects, we can't remove it
       if (EffectAnalyzer(passOptions, curr->value).hasSideEffects()) {
-        noWay = true;
+        foundProblem = true;
       }
     }
   }
@@ -103,20 +103,20 @@ struct ProblemFinder : public ControlFlowWalker<ProblemFinder> {
 
   void visitSwitch(Switch* curr) {
     if (curr->default_ == origin) {
-      noWay = true;
+      foundProblem = true;
       return;
     }
     for (auto& target : curr->targets) {
       if (target == origin) {
-        noWay = true;
+        foundProblem = true;
         return;
       }
     }
   }
 
   bool found() {
     assert(brIfs >= droppedBrIfs);
-    return noWay || brIfs > droppedBrIfs;
+    return foundProblem || brIfs > droppedBrIfs;
   }
 };
 

src/passes/OptimizeInstructions.cpp

@@ -884,15 +884,15 @@ struct OptimizeInstructions : public WalkerPass<PostWalker<OptimizeInstructions,
     if (!Properties::emitsBoolean(left) || !Properties::emitsBoolean(right)) return nullptr;
     auto leftEffects = EffectAnalyzer(getPassOptions(), left);
     auto rightEffects = EffectAnalyzer(getPassOptions(), right);
-    auto leftSideEffects = leftEffects.hasSideEffects();
-    auto rightSideEffects = rightEffects.hasSideEffects();
-    if (leftSideEffects && rightSideEffects) return nullptr; // both must execute
+    auto leftHasSideEffects = leftEffects.hasSideEffects();
+    auto rightHasSideEffects = rightEffects.hasSideEffects();
+    if (leftHasSideEffects && rightHasSideEffects) return nullptr; // both must execute
     // canonicalize with side effects, if any, happening on the left
-    if (rightSideEffects) {
+    if (rightHasSideEffects) {
       if (CostAnalyzer(left).cost < MIN_COST) return nullptr; // avoidable code is too cheap
       if (leftEffects.invalidates(rightEffects)) return nullptr; // cannot reorder
       std::swap(left, right);
-    } else if (leftSideEffects) {
+    } else if (leftHasSideEffects) {
       if (CostAnalyzer(right).cost < MIN_COST) return nullptr; // avoidable code is too cheap
     } else {
       // no side effects, reorder based on cost estimation

src/wasm/wasm-binary.cpp

@@ -2073,7 +2073,7 @@ void WasmBinaryBuilder::visitBlock(Block *curr) {
       auto* item = expressionStack[i];
       curr->list.push_back(item);
       if (i < end - 1) {
-        // stacky&unreachable code may introduce elements that need to be dropped in non-final positoins
+        // stacky&unreachable code may introduce elements that need to be dropped in non-final positions
         if (isConcreteWasmType(item->type)) {
           curr->list.back() = Builder(wasm).makeDrop(curr->list.back());
         }