Fix/Feat clang build (#51)

* chore: format * fix: concepts compile * chore: format * fix: concepts compile * chore: rewrite CI * fix: tests clang build * chore: CI fixes * chore: CI fixes * chore: switch clang to msys [skip fmt] * feat: add boost 1.75.0 to msys CI * fix: segfault on tests with msys/clang * chore: make CI msys only gcc * chore: add clang to ubuntu ci * fix: msvc non tls CI * fix: build after merge * chore: llvm -> clang [skip ci]
Tectu · Jul 8, 2021 · 5d54472 · 5d54472
1 parent 7098f8e
commit 5d54472
Show file tree

Hide file tree

Showing 10 changed files with 266 additions and 259 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -1,71 +1,108 @@
 name: CI
 
-on: 
+on:
   push:
-  pull_request: 
+  pull_request:
     branches: [main]
 
-env: 
-  BUILD_TYPE: Release 
-  INSTALL_LOCATION: .local 
+env:
+  BUILD_TYPE: Release
+  INSTALL_LOCATION: .local
 
 jobs:
   build:
-    strategy: 
+    strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, windows-latest]
-        experimental: [false]
         boost_version: [1.74.0, 1.75.0, 1.76.0]
-        malloy_tls: [ON, OFF]
-        msvc: [false, true]
-        exclude:
-          - os: ubuntu-latest 
-            msvc: false
-          - os: windows-latest  
-            msvc: false # choco package for mingw 10.2.0 does not support threads, see: #21
+        toolchain:
+          [
+            {
+              name: "Ubuntu GCC TLS",
+              cxx: "g++-11",
+              cc: "gcc-11",
+              packages: "libssl-dev gcc-11 g++-11",
+              tls: ON,
+              os: ubuntu-latest,
+            },
+            {
+              name: "Ubuntu GCC",
+              cxx: "g++-11",
+              cc: "gcc-11",
+              packages: "gcc-11 g++-11",
+              tls: OFF,
+              os: ubuntu-latest,
+            },
+            {
+              name: "Ubuntu Clang TLS",
+              cxx: "clang++",
+              cc: "clang",
+              packages: "libssl-dev",
+              tls: ON,
+              os: ubuntu-latest,
+            },
+            {
+              name: "Ubuntu Clang",
+              cxx: "clang++",
+              cc: "clang",
+              packages: "",
+              tls: OFF,
+              os: ubuntu-latest,
+            },
+            {
+              name: "VS2019",
+              cxx: "cl.exe",
+              cc: "cl.exe",
+              tls: OFF,
+              packages: "openssl", # We have to install something
+              os: windows-latest,
+            },
+            {
+              name: "VS2019 TLS",
+              cxx: "cl.exe",
+              cc: "cl.exe",
+              tls: ON,
+              packages: "openssl",
+              os: windows-latest,
+            },
+          ]
 
-    continue-on-error: ${{ matrix.experimental }}
-    runs-on: ${{ matrix.os }}
+    continue-on-error: false
+    runs-on: ${{ matrix.toolchain.os }}
     env:
       BOOST_ROOT: ${{github.workspace}}/3rdparty/boost-${{ matrix.boost_version }}
+      CC: ${{ matrix.toolchain.cc }}
+      CXX: ${{ matrix.toolchain.cxx }}
 
-    name: '${{ matrix.os }}: boost ${{ matrix.boost_version }} tls: ${{ matrix.malloy_tls }}'
+    name: "${{ matrix.toolchain.name }} (boost v${{ matrix.boost_version }})"
     if: "!contains(github.event.head_commit.message, '[ci skip]')"
     steps:
       - uses: actions/checkout@v2
-      
+
       - name: cache boost
-        uses: actions/cache@v2 
+        uses: actions/cache@v2
         id: cache-boost
-        with: 
+        with:
           path: ${{ env.BOOST_ROOT }}
           key: boost-${{ matrix.boost_version }}
-      - name: Install Ninja 
+      - name: Install Ninja
         uses: seanmiddleditch/gha-setup-ninja@master
       - name: Setup boost env
         run: |
           BOOST_URL="https://boostorg.jfrog.io/artifactory/main/release/${{ matrix.boost_version }}/source/boost_$(echo ${{ matrix.boost_version }} | sed 's/\./_/g').tar.bz2"
           echo "BOOST_URL=$BOOST_URL" >> $GITHUB_ENV
         shell: bash
-      - name: install gcc11
-        if: runner.os == 'Linux'
-        run: | 
-          sudo apt install gcc-11 g++-11 -y
-          echo "CC=gcc-11" >> $GITHUB_ENV 
-          echo "CXX=g++-11" >> $GITHUB_ENV 
-
-      - name: Install openssl (via choco)
+      - name: Install packages (via choco)
         if: runner.os == 'Windows'
-        run: choco install openssl 
+        run: choco upgrade ${{ matrix.toolchain.packages }}
 
-      - name: Install openssl (via apt) 
+      - name: Install packages (via apt)
         if: runner.os == 'Linux'
-        run: sudo apt install libssl-dev -y 
+        run: sudo apt install ${{ matrix.toolchain.packages }} -y
 
-      - name: Install openssl (via homebrew)
+      - name: Install packages (via homebrew)
         if: runner.os == 'macOS'
-        run: brew install openssl
+        run: brew install ${{ matrix.toolchain.packages }}
 
       - name: Install Boost
         if: steps.cache-boost.outputs.cache-hit != 'true'
@@ -81,22 +118,16 @@ jobs:
           cd $BOOST_ROOT && cp -r boost_*/* .
           rm -rf boost_*/* download.tar.bz2 download.tar
         shell: bash
-      - name: Install latest mingw
-        if: runner.os == 'Windows' && !matrix.msvc
-        run: choco upgrade mingw 
-      - name: Install latest MSVC 
-        if: runner.os == 'Windows' && matrix.msvc
-        uses: ilammy/msvc-dev-cmd@v1
-
-      - name: Configure 
-        run: cmake -Bbuild -GNinja -DMALLOY_BUILD_EXAMPLES=ON -DMALLOY_BUILD_TESTS=ON -DMALLOY_FEATURE_TLS=${{ matrix.malloy_tls }}
 
-      - name: Build 
-        run: cmake --build build 
-
-      - name: Run tests 
-        run: ./build/test/malloy-tests
+      - name: Install msvc
+        if: ${{ matrix.toolchain.cxx == 'cl.exe' }} # This is a bit of a hack
+        uses: ilammy/msvc-dev-cmd@v1
 
+      - name: Configure
+        run: cmake -Bbuild -GNinja -DMALLOY_BUILD_EXAMPLES=ON -DMALLOY_BUILD_TESTS=ON -DMALLOY_FEATURE_TLS=${{ matrix.toolchain.tls }}
 
-
+      - name: Build
+        run: cmake --build build
 
+      - name: Run tests
+        run: ./build/test/malloy-tests
diff --git a/.github/workflows/msys2.yml b/.github/workflows/msys2.yml
@@ -1,48 +1,47 @@
 name: MSYS2 CI
 
-on: 
+on:
   push:
-  pull_request: 
+  pull_request:
     branches: [main]
 
-env: 
-  BUILD_TYPE: Release 
-  INSTALL_LOCATION: .local 
+env:
+  BUILD_TYPE: Release
+  INSTALL_LOCATION: .local
 
 jobs:
   build:
-    strategy: 
+    strategy:
       fail-fast: false
       matrix:
-        boost_version: [1.74.0, 1.76.0]
+        boost_version: [1.76.0]
         malloy_tls: [ON, OFF]
-        
+
     runs-on: windows-latest
     env:
       BOOST_ROOT: ${{github.workspace}}/3rdparty/boost-${{ matrix.boost_version }}
 
-    name: 'mingw: boost ${{ matrix.boost_version }} tls: ${{ matrix.malloy_tls }}'
+    name: "msys2/clang (boost v${{ matrix.boost_version }}) (tls: ${{ matrix.malloy_tls }})"
     if: "!contains(github.event.head_commit.message, '[ci skip]')"
     defaults:
-        run:
-            shell: msys2 {0}
+      run:
+        shell: msys2 {0}
     steps:
-
       - uses: msys2/setup-msys2@v2
         with:
-            msystem: MINGW64
-            update: false
-            install: >-
-              base-devel
-              mingw-w64-x86_64-toolchain 
-              mingw-w64-x86_64-cmake 
-              p7zip
+          msystem: MINGW64
+          update: false
+          install: >-
+            base-devel
+            mingw-w64-x86_64-toolchain
+            mingw-w64-x86_64-cmake 
+            p7zip
 
       - uses: actions/checkout@v2
       - name: cache boost
-        uses: actions/cache@v2 
+        uses: actions/cache@v2
         id: cache-boost
-        with: 
+        with:
           path: ${{ env.BOOST_ROOT }}
           key: boost-${{ matrix.boost_version }}
       - name: Setup boost env
@@ -65,18 +64,12 @@ jobs:
           7z -o$BOOST_ROOT x $BOOST_ROOT/download.tar -y -bd
           cd $BOOST_ROOT && cp -r boost_*/* .
           rm -rf boost_*/* download.tar.bz2 download.tar
-     
-      - name: Configure 
+
+      - name: Configure
         run: cmake -Bbuild -G"MSYS Makefiles" -DMALLOY_BUILD_EXAMPLES=ON -DMALLOY_BUILD_TESTS=ON -DMALLOY_FEATURE_TLS=${{ matrix.malloy_tls }} -DMALLOY_DEPENDENCY_SPDLOG_DOWNLOAD=ON
 
-      - name: Build 
-        run: cmake --build build 
+      - name: Build
+        run: cmake --build build
 
-      - name: Run tests 
+      - name: Run tests
         run: ./build/test/malloy-tests
-
-
-
-
-
-
diff --git a/lib/malloy/client/type_traits.hpp b/lib/malloy/client/type_traits.hpp
@@ -4,11 +4,25 @@
 #include <concepts>
 #include <variant>
 
+#include "malloy/http/type_traits.hpp"
 #include "malloy/type_traits.hpp"
 
 namespace malloy::client::concepts {
     namespace detail
     {
+        template<typename F>
+        struct response_filter_body_helper {
+            template<malloy::http::concepts::body T>
+            void operator()(T&& v)
+            {
+F f2;
+                typename F::header_type h2;
+                typename std::decay_t<T>::value_type r;
+                f2.setup_body(h2, r);
+            }
+        };
+
+
         /**
          * @class http_cb_helper
          * @brief Helper for http_callback concept
@@ -35,17 +49,14 @@ namespace malloy::client::concepts {
     }
 
     template<typename F>
-    concept response_filter = std::move_constructible<F> && requires(const F& f, const typename F::header_type& h) {
-        { f.body_for(h) } -> malloy::concepts::is_variant;
+    concept response_filter = std::move_constructible<F> && requires(const F& f, const typename F::header_type& h)
+    {
         {
-            std::visit([](auto&& v) {
-                F f2;
-                typename F::header_type h2;
-                typename std::decay_t<decltype(v)>::value_type r;
-                f2.setup_body(h2, r);
-                }, f.body_for(h)) 
-        };
-    }; 
+            f.body_for(h)
+            } -> malloy::concepts::is_variant;
+        {
+            std::visit(detail::response_filter_body_helper<F>{}, f.body_for(h))};
+    };
 
     template<typename F, typename Filter>
     concept http_callback = response_filter<Filter>&& std::move_constructible<F>&& requires(F cb, const Filter& f, const typename Filter::header_type& h){

diff --git a/lib/malloy/server/routing/router.cpp b/lib/malloy/server/routing/router.cpp
@@ -20,11 +20,9 @@ bool router::add_http_endpoint(std::shared_ptr<endpoint_http>&& ep)
 {
     try {
         m_endpoints_http.emplace_back(std::move(ep));
-    }
-    catch (const std::exception& e) {
+    } catch (const std::exception& e) {
         return log_or_throw(e, spdlog::level::critical, "could not add HTTP endpoint: {}", e.what());
-    }
-    catch (...) {
+    } catch (...) {
         return log_or_throw(std::runtime_error("unknown exception"), spdlog::level::critical, "could not add HTTP endpoint. unknown exception.");
     }
 
@@ -35,11 +33,9 @@ bool router::add_websocket_endpoint(std::shared_ptr<endpoint_websocket>&& ep)
 {
     try {
         m_endpoints_websocket.emplace_back(std::move(ep));
-    }
-    catch (const std::exception& e) {
+    } catch (const std::exception& e) {
         return log_or_throw(e, spdlog::level::critical, "could not add WebSocket endpoint: {}", e.what());
-    }
-    catch (...) {
+    } catch (...) {
         return log_or_throw(std::runtime_error("unknown exception"), spdlog::level::critical, "could not add WebSocket endpoint. unknown exception.");
     }
 
@@ -73,16 +69,14 @@ bool router::add_subrouter(std::string resource, std::shared_ptr<router> sub_rou
     // Add router
     try {
         m_sub_routers.try_emplace(std::move(resource), std::move(sub_router));
-    }
-    catch (const std::exception& e) {
+    } catch (const std::exception& e) {
         return log_or_throw(e, spdlog::level::critical, "could not add router: {}", e.what());
     }
 
     return true;
 }
 
 
-
 bool router::add_file_serving(std::string resource, std::filesystem::path storage_base_path)
 {
     // Log
@@ -94,8 +88,8 @@ bool router::add_file_serving(std::string resource, std::filesystem::path storag
 
     ep->resource_base = resource;
     ep->base_path = std::move(storage_base_path);
-    ep->writer = [](const auto& req, auto&& resp, const auto& conn) { 
-        std::visit([&](auto&& resp) { detail::send_response(req, std::move(resp), conn);  }, std::move(resp));
+    ep->writer = [](const auto& req, auto&& resp, const auto& conn) {
+        std::visit([&](auto&& resp) { detail::send_response(req, std::move(resp), conn); }, std::move(resp));
     };
 
     // Add
@@ -154,14 +148,14 @@ router::response_type router::generate_preflight_response(const request_header&
 
     // Create a string representing all supported methods
     std::string methods_string;
-    for (const auto &str : method_strings) {
+    for (const auto& str : method_strings) {
         methods_string += str;
         if (&str not_eq
             &method_strings.back())
             methods_string += ", ";
     }
 
-    malloy::http::response<> resp{ malloy::http::status::ok };
+    malloy::http::response<> resp{malloy::http::status::ok};
     resp.set(boost::beast::http::field::content_type, "text/html");
     resp.base().set("Access-Control-Allow-Origin", "http://127.0.0.1:8080");
     resp.base().set("Access-Control-Allow-Methods", methods_string);
@@ -170,4 +164,3 @@ router::response_type router::generate_preflight_response(const request_header&
 
     return resp;
 }
-