Sanitizer (#644)

* Add sanitizer coverage to CMake and CI.
* Fix bug found by sanitizer
* Add GCC and Clangformat to CI.
* Enable workflow dispatch

.github/workflows/cmake.yml

@@ -5,6 +5,7 @@ on:
     branches: [ "master" ]
   pull_request:
     branches: [ "master" ]
+  workflow_dispatch:
 
 jobs:
   build-test:
@@ -13,23 +14,48 @@ jobs:
         platform: [ "ubuntu-latest", "macos-latest", "windows-latest" ]
         build-type: [ "Release", "Debug" ]
 
+        # Extra cmake flags.  GitHub Actions matrix overloads `include` to mean
+        # 'add extra things to a job' and 'add jobs'.  You can add extra things
+        # to a job by specifying things that exist in a job created from the
+        # matrix definition and adding things.  You can specify extra jobs by
+        # specifying properties that don't match existing jobs.  We use
+        # `cmake-flags` to add cmake flags to all jobs matching a pattern and
+        # `extra-cmake-flags` to specify a new job with custom CMake flags.
+        extra-cmake-flags: [ "" ]
+
         include:
         - platform: "ubuntu-latest"
           cmake-flags: "-G Ninja -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang"
           dependencies: "sudo apt install ninja-build"
+        - platform: "ubuntu-latest"
+          variant: "GCC"
+          cmake-flags: "-G Ninja"
+          dependencies: "sudo apt install ninja-build"
+        - platform: "ubuntu-latest"
+          variant: "asan"
+          build-type: "Release"
+          cmake-flags: "-G Ninja -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang"
+          extra-cmake-flags: "-DVERONA_SANITIZER=address"
+          dependencies: "sudo apt install ninja-build"
+        - platform: "ubuntu-latest"
+          variant: "ubsan"
+          build-type: "Debug"
+          cmake-flags: "-G Ninja -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang"
+          extra-cmake-flags: "-DVERONA_SANITIZER=undefined"
+          dependencies: "sudo apt install ninja-build"
 
       fail-fast: false
 
     runs-on: ${{matrix.platform}}
-
+    name: ${{ matrix.platform }} ${{ matrix.build-type }} ${{ matrix.variant }}
     steps:
     - uses: actions/checkout@v3
 
     - name: Install build dependencies
       run: ${{ matrix.dependencies }}
 
     - name: Configure CMake
-      run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} ${{matrix.cmake-flags}}
+      run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} ${{matrix.cmake-flags}} ${{matrix.extra-cmake-flags}}
 
     - name: Build
       # Build your program with the given configuration
@@ -41,3 +67,29 @@ jobs:
       # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
       run: ctest -C ${{matrix.build-type}} --output-on-failure --timeout 400 --interactive-debug-mode 0 
 
+  # Job to run clang-format and report errors
+  format:
+    runs-on: ubuntu-22.04
+    # We don't need to do the build for this job, but we need to configure it to get the clang-format target
+    steps:
+    - uses: actions/checkout@v3
+    - name: Install clang-tidy and clang-format
+      run: |
+        sudo apt update
+        sudo apt install clang-tidy-15 clang-format-15
+    - name: Configure CMake
+      run: cmake -B ${{github.workspace}}/build
+    # Run the clang-format check and error if it generates a diff
+    - name: Run clang-format
+      working-directory: ${{github.workspace}}/build
+      run: |
+        set -eo pipefail
+        make clangformat
+        git diff --exit-code
+
+  all-checks:
+    needs: [format, build-test]
+    runs-on: ubuntu-latest
+    steps:
+    - name: Dummy step
+      run: true

CMakeLists.txt

@@ -17,6 +17,9 @@ FetchContent_Declare(
 
 FetchContent_MakeAvailable(trieste)
 
+# Use snmalloc clangformat target
+clangformat_targets()
+
 FetchContent_Declare(
   fmt
   GIT_REPOSITORY https://github.com/fmtlib/fmt

src/CMakeLists.txt

@@ -53,6 +53,15 @@ target_link_libraries(verona
   trieste::trieste
 )
 
+if (VERONA_SANITIZER)
+  target_compile_options(verona PUBLIC
+    -fsanitize=${VERONA_SANITIZER} -g -fno-omit-frame-pointer
+  )
+  target_link_options(verona PUBLIC
+    -fsanitize=${VERONA_SANITIZER}
+  )
+endif()
+
 add_test(NAME verona COMMAND verona test -f)
 
 install(TARGETS verona RUNTIME DESTINATION verona)

src/btype.h

@@ -57,7 +57,8 @@ namespace verona
           // If it's bound to itself, check the next binding.
           if (it->second->type() == TypeParamBind)
           {
-            for (auto& bind : it->second->bindings)
+            auto bound = it->second;
+            for (auto& bind : bound->bindings)
               bindings[bind.first] = bind.second;
 
             it = bindings.find(node);

src/parse.cc

@@ -57,13 +57,13 @@ namespace verona
       return RE2::FullMatch(path.filename().string(), *re_dir);
     });
 
-    p.postparse([](auto& p, auto& path, auto ast) {
+    p.postparse([](auto& pp, auto& path, auto ast) {
       if (options().no_std)
         return;
 
-      auto stdlib = p.executable().parent_path() / "std";
+      auto stdlib = pp.executable().parent_path() / "std";
       if (path != stdlib)
-        ast << p.sub_parse(stdlib);
+        ast << pp.sub_parse(stdlib);
     });
 
     p.postfile([indent, depth](auto&, auto&, auto) {

src/passes/drop.cc

@@ -17,7 +17,7 @@ namespace verona
     NodeMap<Nodes> successors;
     bool llvm;
 
-    track(bool llvm) : llvm(llvm) {}
+    track(bool llvm_) : llvm(llvm_) {}
 
     void gen(const Location& loc)
     {

src/passes/traitisect.cc

@@ -19,8 +19,7 @@ namespace verona
           [](Match& _) {
             // If we're inside a TypeIsect, put the new traits inside it.
             // Otherwise, create a new TypeIsect.
-            Node r =
-              (_(Trait)->parent() == TypeIsect) ? Seq : TypeIsect;
+            Node r = (_(Trait)->parent() == TypeIsect) ? Seq : TypeIsect;
 
             Node base = ClassBody;
             r << (Trait << _(Ident) << base);

src/subtype.cc

@@ -32,7 +32,7 @@ namespace verona
     Btype sub;
     Btype sup;
 
-    Assume(Btype sub, Btype sup) : sub(sub), sup(sup)
+    Assume(Btype sub_, Btype sup_) : sub(sub_), sup(sup_)
     {
       assert(sub->in({Class, Trait}));
       assert(sup == Trait);