Modernize code to C++17 (#3104)

Author: MaxGraey

CMakeLists.txt

@@ -165,7 +165,7 @@ if(MSVC)
       add_compile_flag("/arch:sse2")
     endif()
   endif()
-  add_compile_flag("/std:c++17") # VS2017 generates "error C2429: attribute 'fallthrough' requires compiler flag '/std:c++17'"
+  add_compile_flag("/std:c++${CXX_STANDARD}")
   add_compile_flag("/wd4146") # Ignore warning "warning C4146: unary minus operator applied to unsigned type, result still unsigned", this pattern is used somewhat commonly in the code.
   # 4267 and 4244 are conversion/truncation warnings. We might want to fix these but they are currently pervasive.
   add_compile_flag("/wd4267")

src/cfg/Relooper.cpp

@@ -62,9 +62,7 @@ static wasm::Expression* HandleFollowupMultiples(wasm::Expression* Ret,
     if (!Multiple) {
       break;
     }
-    for (auto& iter : Multiple->InnerMap) {
-      int Id = iter.first;
-      Shape* Body = iter.second;
+    for (auto& [Id, Body] : Multiple->InnerMap) {
       Curr->name = Builder.getBlockBreakName(Id);
       Curr->finalize(); // it may now be reachable, via a break
       auto* Outer = Builder.makeBlock(Curr);
@@ -222,13 +220,13 @@ wasm::Expression* Block::Render(RelooperBuilder& Builder, bool InLoop) {
   Block* DefaultTarget = nullptr;
 
   // Find the default target, the one without a condition
-  for (auto& iter : ProcessedBranchesOut) {
-    if ((!SwitchCondition && !iter.second->Condition) ||
-        (SwitchCondition && !iter.second->SwitchValues)) {
+  for (auto& [Target, Details] : ProcessedBranchesOut) {
+    if ((!SwitchCondition && !Details->Condition) ||
+        (SwitchCondition && !Details->SwitchValues)) {
       assert(!DefaultTarget &&
              "block has branches without a default (nullptr for the "
              "condition)"); // Must be exactly one default // nullptr
-      DefaultTarget = iter.first;
+      DefaultTarget = Target;
     }
   }
   // Since each Block* must* branch somewhere, this must be set
@@ -338,9 +336,7 @@ wasm::Expression* Block::Render(RelooperBuilder& Builder, bool InLoop) {
     auto* Outer = Builder.makeBlock();
     auto* Inner = Outer;
     std::vector<wasm::Name> Table;
-    for (auto& iter : ProcessedBranchesOut) {
-      Block* Target = iter.first;
-      Branch* Details = iter.second;
+    for (auto& [Target, Details] : ProcessedBranchesOut) {
       wasm::Name CurrName;
       if (Details->SwitchValues) {
         CurrName = wasm::Name(Base + "$case$" + std::to_string(Target->Id));
@@ -435,9 +431,9 @@ wasm::Expression* MultipleShape::Render(RelooperBuilder& Builder, bool InLoop) {
   wasm::If* FirstIf = nullptr;
   wasm::If* CurrIf = nullptr;
   std::vector<wasm::If*> finalizeStack;
-  for (auto& iter : InnerMap) {
-    auto* Now = Builder.makeIf(Builder.makeCheckLabel(iter.first),
-                               iter.second->Render(Builder, InLoop));
+  for (auto& [Id, Body] : InnerMap) {
+    auto* Now =
+      Builder.makeIf(Builder.makeCheckLabel(Id), Body->Render(Builder, InLoop));
     finalizeStack.push_back(Now);
     if (!CurrIf) {
       FirstIf = CurrIf = Now;
@@ -610,8 +606,7 @@ struct Optimizer : public RelooperRecursor {
   void CanonicalizeCode() {
     for (auto& Block : Parent->Blocks) {
       Block->Code = Canonicalize(Block->Code);
-      for (auto& iter : Block->BranchesOut) {
-        auto* Branch = iter.second;
+      for (auto& [_, Branch] : Block->BranchesOut) {
         if (Branch->Code) {
           Branch->Code = Canonicalize(Branch->Code);
         }
@@ -647,15 +642,14 @@ struct Optimizer : public RelooperRecursor {
               Next = Replacement = NextNext;
               // If we've already seen this, stop - it's an infinite loop of
               // empty blocks we can skip through.
-              if (Seen.count(Replacement)) {
+              if (!Seen.emplace(Replacement).second) {
                 // Stop here. Note that if we started from X and ended up with X
                 // once more, then Replacement == First and so lower down we
                 // will not report that we did any work, avoiding an infinite
                 // loop due to always thinking there is more work to do.
                 break;
               } else {
                 // Otherwise, keep going.
-                Seen.insert(Replacement);
                 continue;
               }
             }
@@ -698,12 +692,10 @@ struct Optimizer : public RelooperRecursor {
       if (ParentBlock->BranchesOut.size() >= 2) {
         std::unordered_map<size_t, std::vector<BranchBlock>> HashedBranchesOut;
         std::vector<Block*> BlocksToErase;
-        for (auto& iter : ParentBlock->BranchesOut) {
-          Block* CurrBlock = iter.first;
+        for (auto& [CurrBlock, CurrBranch] : ParentBlock->BranchesOut) {
 #if RELOOPER_OPTIMIZER_DEBUG
           std::cout << "  consider child " << CurrBlock->Id << '\n';
 #endif
-          Branch* CurrBranch = iter.second;
           if (CurrBranch->Code) {
             // We can't merge code; ignore
             continue;
@@ -712,9 +704,7 @@ struct Optimizer : public RelooperRecursor {
           auto& HashedSiblings = HashedBranchesOut[HashValue];
           // Check if we are equivalent to any of them - if so, merge us.
           bool Merged = false;
-          for (auto& Pair : HashedSiblings) {
-            Branch* SiblingBranch = Pair.first;
-            Block* SiblingBlock = Pair.second;
+          for (auto& [SiblingBranch, SiblingBlock] : HashedSiblings) {
             if (HaveEquivalentContents(CurrBlock, SiblingBlock)) {
 #if RELOOPER_OPTIMIZER_DEBUG
               std::cout << "    equiv! to " << SiblingBlock->Id << '\n';
@@ -809,8 +799,8 @@ struct Optimizer : public RelooperRecursor {
       } else {
         // If the block has no switch, the branches must not as well.
 #ifndef NDEBUG
-        for (auto& iter : ParentBlock->BranchesOut) {
-          assert(!iter.second->SwitchValues);
+        for (auto& [_, CurrBranch] : ParentBlock->BranchesOut) {
+          assert(!CurrBranch->SwitchValues);
         }
 #endif
       }
@@ -923,9 +913,7 @@ struct Optimizer : public RelooperRecursor {
     if (A->BranchesOut.size() != B->BranchesOut.size()) {
       return false;
     }
-    for (auto& aiter : A->BranchesOut) {
-      Block* ABlock = aiter.first;
-      Branch* ABranch = aiter.second;
+    for (auto& [ABlock, ABranch] : A->BranchesOut) {
       if (B->BranchesOut.count(ABlock) == 0) {
         return false;
       }
@@ -1030,11 +1018,11 @@ struct Optimizer : public RelooperRecursor {
                          wasm::ExpressionAnalyzer::hash(Curr->SwitchCondition));
     }
     wasm::rehash(digest, uint8_t(2));
-    for (auto& Pair : Curr->BranchesOut) {
+    for (auto& [CurrBlock, CurrBranch] : Curr->BranchesOut) {
       // Hash the Block* as a pointer TODO: full hash?
-      wasm::rehash(digest, reinterpret_cast<size_t>(Pair.first));
+      wasm::rehash(digest, reinterpret_cast<size_t>(CurrBlock));
       // Hash the Branch info properly
-      wasm::hash_combine(digest, Hash(Pair.second));
+      wasm::hash_combine(digest, Hash(CurrBranch));
     }
     return digest;
   }
@@ -1077,8 +1065,8 @@ void Relooper::Calculate(Block* Entry) {
     if (!contains(Live.Live, Curr)) {
       continue;
     }
-    for (auto& iter : Curr->BranchesOut) {
-      iter.first->BranchesIn.insert(Curr);
+    for (auto& [CurrBlock, _] : Curr->BranchesOut) {
+      CurrBlock->BranchesIn.insert(Curr);
     }
   }
 
@@ -1092,9 +1080,9 @@ void Relooper::Calculate(Block* Entry) {
     void GetBlocksOut(Block* Source,
                       BlockSet& Entries,
                       BlockSet* LimitTo = nullptr) {
-      for (auto& iter : Source->BranchesOut) {
-        if (!LimitTo || contains(*LimitTo, iter.first)) {
-          Entries.insert(iter.first);
+      for (auto& [CurrBlock, _] : Source->BranchesOut) {
+        if (!LimitTo || contains(*LimitTo, CurrBlock)) {
+          Entries.insert(CurrBlock);
         }
       }
     }
@@ -1174,8 +1162,7 @@ void Relooper::Calculate(Block* Entry) {
       assert(InnerBlocks.size() > 0);
 
       for (auto* Curr : InnerBlocks) {
-        for (auto& iter : Curr->BranchesOut) {
-          Block* Possible = iter.first;
+        for (auto& [Possible, _] : Curr->BranchesOut) {
           if (!contains(InnerBlocks, Possible)) {
             NextEntries.insert(Possible);
           }
@@ -1289,8 +1276,7 @@ void Relooper::Calculate(Block* Entry) {
             // may have been seen before and invalidated already
             if (Ownership[Invalidatee]) {
               Ownership[Invalidatee] = nullptr;
-              for (auto& iter : Invalidatee->BranchesOut) {
-                Block* Target = iter.first;
+              for (auto& [Target, _] : Invalidatee->BranchesOut) {
                 auto Known = Ownership.find(Target);
                 if (Known != Ownership.end()) {
                   Block* TargetOwner = Known->second;
@@ -1330,8 +1316,7 @@ void Relooper::Calculate(Block* Entry) {
           continue;
         }
         // Add all children
-        for (auto& iter : Curr->BranchesOut) {
-          Block* New = iter.first;
+        for (auto& [New, _] : Curr->BranchesOut) {
           auto Known = Helper.Ownership.find(New);
           if (Known == Helper.Ownership.end()) {
             // New node. Add it, and put it in the queue
@@ -1404,9 +1389,7 @@ void Relooper::Calculate(Block* Entry) {
                  IndependentGroups.size());
       MultipleShape* Multiple = Parent->AddMultipleShape();
       BlockSet CurrEntries;
-      for (auto& iter : IndependentGroups) {
-        Block* CurrEntry = iter.first;
-        BlockSet& CurrBlocks = iter.second;
+      for (auto& [CurrEntry, CurrBlocks] : IndependentGroups) {
         PrintDebug("  multiple group with entry %d:\n", CurrEntry->Id);
         DebugDump(CurrBlocks, "    ");
         // Create inner block
@@ -1563,8 +1546,7 @@ void Relooper::Calculate(Block* Entry) {
               bool DeadEnd = true;
               BlockSet& SmallGroup = IndependentGroups[SmallEntry];
               for (auto* Curr : SmallGroup) {
-                for (auto& iter : Curr->BranchesOut) {
-                  Block* Target = iter.first;
+                for (auto& [Target, _] : Curr->BranchesOut) {
                   if (!contains(SmallGroup, Target)) {
                     DeadEnd = false;
                     break;
@@ -1685,8 +1667,8 @@ void Debugging::Dump(Shape* S, const char* prefix) {
     printf("<< Simple with block %d\n", Simple->Inner->Id);
   } else if (MultipleShape* Multiple = Shape::IsMultiple(S)) {
     printf("<< Multiple\n");
-    for (auto& iter : Multiple->InnerMap) {
-      printf("     with entry %d\n", iter.first);
+    for (auto& [Entry, _] : Multiple->InnerMap) {
+      printf("     with entry %d\n", Entry);
     }
   } else if (Shape::IsLoop(S)) {
     printf("<< Loop\n");

src/cfg/cfg-traversal.h

@@ -536,8 +536,8 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
   void checkDuplicates(std::vector<BasicBlock*>& list) {
     std::unordered_set<BasicBlock*> seen;
     for (auto* curr : list) {
-      assert(seen.count(curr) == 0);
-      seen.insert(curr);
+      auto res = seen.emplace(curr);
+      assert(res.second);
     }
   }
 

src/dataflow/graph.h

@@ -31,9 +31,7 @@
 #include "ir/literal-utils.h"
 #include "wasm.h"
 
-namespace wasm {
-
-namespace DataFlow {
+namespace wasm::DataFlow {
 
 // Main logic to generate IR for a function. This is implemented as a
 // visitor on the wasm, where visitors return a Node* that either
@@ -797,8 +795,6 @@ struct Graph : public UnifiedExpressionVisitor<Graph, Node*> {
   const Name FAKE_CALL = "fake$dfo$call";
 };
 
-} // namespace DataFlow
-
-} // namespace wasm
+} // namespace wasm::DataFlow
 
 #endif // wasm_dataflow_graph_h

src/dataflow/node.h

@@ -28,9 +28,7 @@
 #include "ir/utils.h"
 #include "wasm.h"
 
-namespace wasm {
-
-namespace DataFlow {
+namespace wasm::DataFlow {
 
 //
 // The core IR representation in DataFlow: a Node.
@@ -218,8 +216,6 @@ struct Node {
   }
 };
 
-} // namespace DataFlow
-
-} // namespace wasm
+} // namespace wasm::DataFlow
 
 #endif // wasm_dataflow_node

src/dataflow/users.h

@@ -27,9 +27,7 @@
 
 #include "dataflow/graph.h"
 
-namespace wasm {
-
-namespace DataFlow {
+namespace wasm::DataFlow {
 
 // Calculates the users of each node.
 //   users[x] = { y, z, .. }
@@ -95,8 +93,6 @@ class Users {
   void removeAllUsesOf(Node* node) { users.erase(node); }
 };
 
-} // namespace DataFlow
-
-} // namespace wasm
+} // namespace wasm::DataFlow
 
 #endif // wasm_dataflow_users

src/dataflow/utils.h

@@ -29,9 +29,7 @@
 #include "dataflow/node.h"
 #include "wasm.h"
 
-namespace wasm {
-
-namespace DataFlow {
+namespace wasm::DataFlow {
 
 inline std::ostream& dump(Node* node, std::ostream& o, size_t indent = 0) {
   auto doIndent = [&]() {
@@ -147,8 +145,6 @@ inline bool allInputsConstant(Node* node) {
   return false;
 }
 
-} // namespace DataFlow
-
-} // namespace wasm
+} // namespace wasm::DataFlow
 
 #endif // wasm_dataflow_utils

src/ir/ExpressionManipulator.cpp

@@ -17,9 +17,7 @@
 #include "ir/load-utils.h"
 #include "ir/utils.h"
 
-namespace wasm {
-
-namespace ExpressionManipulator {
+namespace wasm::ExpressionManipulator {
 
 Expression*
 flexibleCopy(Expression* original, Module& wasm, CustomCopier custom) {
@@ -118,6 +116,4 @@ void spliceIntoBlock(Block* block, Index index, Expression* add) {
   block->finalize(block->type);
 }
 
-} // namespace ExpressionManipulator
-
-} // namespace wasm
+} // namespace wasm::ExpressionManipulator