Update from C++17 to C++20 (#8218)

As a test I converted all the `set::count` calls to `set::contains` when
they were used for membership testing.

Author: sbc100

.github/workflows/ci.yml

@@ -383,40 +383,6 @@ jobs:
     - name: build
       run: cmake --build out
 
-  # Duplicates build-asan.  Please keep in sync
-  build-cxx20:
-    name: c++20
-    # Make sure we can still build on older Ubuntu
-    runs-on: ubuntu-22.04
-    env:
-      CC: "gcc"
-      CXX: "g++"
-    steps:
-    - uses: actions/setup-python@v5
-      with:
-        python-version: '3.x'
-    - uses: actions/checkout@v4
-      with:
-        submodules: true
-    - name: install ninja
-      run: sudo apt-get install ninja-build
-    - name: install v8
-      run: |
-        npm install jsvu -g
-        jsvu --os=default --engines=v8
-    - name: install Python dev dependencies
-      run: pip3 install -r requirements-dev.txt
-    - name: cmake
-      run: |
-        mkdir -p out
-        cmake -S . -B out -G Ninja -DCMAKE_INSTALL_PREFIX=out/install -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20
-    - name: build
-      run: cmake --build out
-    - name: test
-      run: |
-        python check.py --binaryen-bin=out/bin lit
-        python check.py --binaryen-bin=out/bin gtest
-
   # Ensures we can build in no-asserts mode (just building).
   build-noasserts:
     name: noasserts

CMakeLists.txt

@@ -11,7 +11,7 @@ include(GNUInstallDirs)
 # The C++ standard whose features are required to build Binaryen.
 # Keep in sync with scripts/test/shared.py cxx_standard
 # The if condition allows embedding in a project with a higher default C++ standard set
-set(REQUIRED_CXX_STANDARD 17)
+set(REQUIRED_CXX_STANDARD 20)
 if(NOT CMAKE_CXX_STANDARD)
   set(CMAKE_CXX_STANDARD ${REQUIRED_CXX_STANDARD})
 elseif(CMAKE_CXX_STANDARD LESS ${REQUIRED_CXX_STANDARD})

README.md

@@ -412,7 +412,9 @@ After that you can build with CMake:
 cmake . && make
 ```
 
-A C++17 compiler is required. On macOS, you need to install `cmake`, for example, via `brew install cmake`. Note that you can also use `ninja` as your generator: `cmake -G Ninja . && ninja`.
+A C++20 compiler is required. On macOS, you need to install `cmake`, for
+example, via `brew install cmake`. Note that you can also use `ninja` as your
+generator: `cmake -G Ninja . && ninja`.
 
 To avoid the gtest dependency, you can pass `-DBUILD_TESTS=OFF` to cmake.
 

scripts/test/shared.py

@@ -27,7 +27,7 @@
 
 # The C++ standard whose features are required to build Binaryen.
 # Keep in sync with CMakeLists.txt CXX_STANDARD
-cxx_standard = 17
+cxx_standard = 20
 
 
 def parse_args(args):

src/binaryen-c.cpp

@@ -5816,7 +5816,7 @@ void BinaryenClearPassArguments(void) { globalPassOptions.arguments.clear(); }
 
 bool BinaryenHasPassToSkip(const char* pass) {
   assert(pass);
-  return globalPassOptions.passesToSkip.count(pass);
+  return globalPassOptions.passesToSkip.contains(pass);
 }
 
 void BinaryenAddPassToSkip(const char* pass) {
@@ -5873,7 +5873,7 @@ void BinaryenModuleRunPasses(BinaryenModuleRef module,
   passRunner.options = globalPassOptions;
   for (BinaryenIndex i = 0; i < numPasses; i++) {
     passRunner.add(passes[i],
-                   globalPassOptions.arguments.count(passes[i]) > 0
+                   globalPassOptions.arguments.contains(passes[i])
                      ? globalPassOptions.arguments[passes[i]]
                      : std::optional<std::string>());
   }
@@ -6125,7 +6125,7 @@ void BinaryenFunctionRunPasses(BinaryenFunctionRef func,
   passRunner.options = globalPassOptions;
   for (BinaryenIndex i = 0; i < numPasses; i++) {
     passRunner.add(passes[i],
-                   globalPassOptions.arguments.count(passes[i]) > 0
+                   globalPassOptions.arguments.contains(passes[i])
                      ? globalPassOptions.arguments[passes[i]]
                      : std::optional<std::string>());
   }

src/cfg/Relooper.cpp

@@ -40,7 +40,7 @@ namespace CFG {
 
 template<class T, class U>
 static bool contains(const T& container, const U& contained) {
-  return !!container.count(contained);
+  return container.contains(contained);
 }
 
 // Rendering utilities
@@ -667,7 +667,7 @@ struct Optimizer : public RelooperRecursor {
         // Add a branch to the target (which may be the unchanged original) in
         // the set of new branches. If it's a replacement, it may collide, and
         // we need to merge.
-        if (NewBranchesOut.count(Replacement)) {
+        if (NewBranchesOut.contains(Replacement)) {
 #if RELOOPER_OPTIMIZER_DEBUG
           std::cout << "  merge\n";
 #endif
@@ -915,7 +915,7 @@ struct Optimizer : public RelooperRecursor {
       return false;
     }
     for (auto& [ABlock, ABranch] : A->BranchesOut) {
-      if (B->BranchesOut.count(ABlock) == 0) {
+      if (!B->BranchesOut.contains(ABlock)) {
         return false;
       }
       auto* BBranch = B->BranchesOut[ABlock];
@@ -1578,7 +1578,7 @@ void Relooper::Calculate(Block* Entry) {
             // jumped to forward, without using the label variable
             bool Checked = false;
             for (auto* Entry : *Entries) {
-              if (InitialEntries.count(Entry)) {
+              if (InitialEntries.contains(Entry)) {
                 Checked = true;
                 break;
               }

src/cfg/cfg-traversal.h

@@ -616,7 +616,7 @@ struct CFGWalker : public PostWalker<SubType, VisitorType> {
       queue.erase(iter);
       alive.insert(curr);
       for (auto* out : curr->out) {
-        if (!alive.count(out)) {
+        if (!alive.contains(out)) {
           queue.insert(out);
         }
       }
@@ -626,21 +626,21 @@ struct CFGWalker : public PostWalker<SubType, VisitorType> {
 
   void unlinkDeadBlocks(std::unordered_set<BasicBlock*> alive) {
     for (auto& block : basicBlocks) {
-      if (!alive.count(block.get())) {
+      if (!alive.contains(block.get())) {
         block->in.clear();
         block->out.clear();
         continue;
       }
       block->in.erase(std::remove_if(block->in.begin(),
                                      block->in.end(),
                                      [&alive](BasicBlock* other) {
-                                       return !alive.count(other);
+                                       return !alive.contains(other);
                                      }),
                       block->in.end());
       block->out.erase(std::remove_if(block->out.begin(),
                                       block->out.end(),
                                       [&alive](BasicBlock* other) {
-                                        return !alive.count(other);
+                                        return !alive.contains(other);
                                       }),
                        block->out.end());
     }
@@ -664,17 +664,17 @@ struct CFGWalker : public PostWalker<SubType, VisitorType> {
     std::cout << "<==\nCFG [" << message << "]:\n";
     generateDebugIds();
     for (auto& block : basicBlocks) {
-      assert(debugIds.count(block.get()) > 0);
+      assert(debugIds.contains(block.get()));
       std::cout << "  block " << debugIds[block.get()] << " (" << block.get()
                 << "):\n";
       block->contents.dump(static_cast<SubType*>(this)->getFunction());
       for (auto& in : block->in) {
-        assert(debugIds.count(in) > 0);
+        assert(debugIds.contains(in));
         assert(std::find(in->out.begin(), in->out.end(), block.get()) !=
                in->out.end()); // must be a parallel link back
       }
       for (auto& out : block->out) {
-        assert(debugIds.count(out) > 0);
+        assert(debugIds.contains(out));
         std::cout << "    out: " << debugIds[out] << "\n";
         assert(std::find(out->in.begin(), out->in.end(), block.get()) !=
                out->in.end()); // must be a parallel link back

src/cfg/liveness-traversal.h

@@ -222,7 +222,7 @@ struct LivenessWalker : public CFGWalker<SubType, VisitorType, Liveness> {
     // keep working while stuff is flowing
     std::unordered_set<BasicBlock*> queue;
     for (auto& curr : CFGWalker<SubType, VisitorType, Liveness>::basicBlocks) {
-      if (liveBlocks.count(curr.get()) == 0) {
+      if (!liveBlocks.contains(curr.get())) {
         continue; // ignore dead blocks
       }
       queue.insert(curr.get());

src/ir/ExpressionAnalyzer.cpp

@@ -353,7 +353,7 @@ struct Hasher {
     //   (block $x (br $x))
     //   (block $y (br $y))
     // But if the name is not known to us, hash the absolute one.
-    if (!internalNames.count(curr)) {
+    if (!internalNames.contains(curr)) {
       rehash(digest, 1);
       // Perform the same hashing as a generic name.
       visitNonScopeName(curr);

src/ir/LocalGraph.cpp

@@ -341,7 +341,7 @@ struct LocalGraphFlower
     auto index = get->index;
 
     // We must never repeat work.
-    assert(!getSetsMap.count(get));
+    assert(!getSetsMap.contains(get));
 
     // Regardless of what we do below, ensure an entry for this get, so that we
     // know we computed it.
@@ -410,7 +410,7 @@ struct LocalGraphFlower
     auto index = set->index;
 
     // We must never repeat work.
-    assert(!setInfluences.count(set));
+    assert(!setInfluences.contains(set));
 
     // In theory we could flow the set forward, but to keep things simple we
     // reuse the logic for flowing gets backwards: We flow all the gets of the
@@ -419,7 +419,7 @@ struct LocalGraphFlower
     // doing work for local indexes we don't care about.
     for (auto* get : getsByIndex[index]) {
       // Don't repeat work.
-      if (!getSetsMap.count(get)) {
+      if (!getSetsMap.contains(get)) {
         computeGetSets(get);
       }
     }
@@ -632,7 +632,7 @@ void LocalGraph::computeSSAIndexes() {
   }
 }
 
-bool LocalGraph::isSSA(Index x) { return SSAIndexes.count(x); }
+bool LocalGraph::isSSA(Index x) { return SSAIndexes.contains(x); }
 
 // LazyLocalGraph
 
@@ -672,7 +672,7 @@ LazyLocalGraph::~LazyLocalGraph() {
 
 void LazyLocalGraph::computeGetSets(LocalGet* get) const {
   // We must never repeat work.
-  assert(!getSetsMap.count(get));
+  assert(!getSetsMap.contains(get));
 
   if (!flower) {
     makeFlower();
@@ -682,7 +682,7 @@ void LazyLocalGraph::computeGetSets(LocalGet* get) const {
 
 void LazyLocalGraph::computeSetInfluences(LocalSet* set) const {
   // We must never repeat work.
-  assert(!setInfluences.count(set));
+  assert(!setInfluences.contains(set));
 
   if (!flower) {
     makeFlower();
@@ -705,7 +705,7 @@ void LazyLocalGraph::computeGetInfluences() const {
 
 bool LazyLocalGraph::computeSSA(Index index) const {
   // We must never repeat work.
-  assert(!SSAIndexes.count(index));
+  assert(!SSAIndexes.contains(index));
 
   if (!flower) {
     makeFlower();