Adding optional usage of C++ 17 (#67)

This commit makes a small set of changes which enable the library to be built in C++ 17 with extremely strict flags. No functionality is affected and the C++ 17 path is entirely optional.

---------

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

.github/workflows/buildtest.yml

@@ -12,11 +12,17 @@ jobs:
       matrix:
         platform: [ "ubuntu-latest", "macos-latest", "windows-latest" ]
         build-type: [ "Release", "Debug" ]
+        standard: [ "", "-DTRIESTE_USE_CXX17=ON" ]
+        compiler: [ "", "-DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang" ]
 
         include:
         - platform: "ubuntu-latest"
-          cmake-flags: "-G Ninja -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang"
+          generator: "-G Ninja"
           dependencies: "sudo apt install ninja-build"
+        - platform: "macos-latest"
+          compiler: ""
+        - platform: "windows-latest"
+          compiler: ""
 
       # Don't abort runners if a single one fails
       fail-fast: false
@@ -30,7 +36,7 @@ jobs:
       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.generator}} ${{matrix.standard}} ${{matrix.compiler}}
 
     - name: Build
       run: cmake --build ${{github.workspace}}/build --config ${{matrix.build-type}}

CMakeLists.txt

@@ -1,9 +1,24 @@
 cmake_minimum_required(VERSION 3.14.0)
 project(trieste VERSION 1.0.0 LANGUAGES CXX)
 
+# #############################################
+# Options
+option(TRIESTE_BUILD_SAMPLES "Specifies whether to build the samples" ON)
+option(TRIESTE_USE_CXX17 "Specifies whether to target the C++17 standard" OFF)
+
+set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
+
+# #############################################
+# Dependencies
+
 include(FetchContent)
 
-set(CMAKE_CXX_STANDARD 20)
+if(TRIESTE_USE_CXX17)
+  set(CMAKE_CXX_STANDARD 17)
+else()
+  set(CMAKE_CXX_STANDARD 20)
+endif()
+
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 
 set(SNMALLOC_BUILD_TESTING OFF CACHE INTERNAL "Turn off snmalloc tests")
@@ -12,6 +27,7 @@ if (MSVC)
 else()
   set(SNMALLOC_STATIC_LIBRARY_PREFIX "")
 endif()
+set(SNMALLOC_USE_CXX17 ${TRIESTE_USE_CXX17})
 
 set(RE2_BUILD_TESTING OFF CACHE INTERNAL "Turn off RE2 tests")
 
@@ -42,12 +58,6 @@ FetchContent_Declare(
 
 FetchContent_MakeAvailable(cli11)
 
-# #############################################
-# Options
-option(TRIESTE_BUILD_SAMPLES "Specifies whether to build the samples" ON)
-
-set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
-
 # #############################################
 # Create target and set properties
 add_library(trieste INTERFACE)
@@ -68,7 +78,11 @@ target_link_libraries(trieste
   CLI11::CLI11
 )
 
-target_compile_features(trieste INTERFACE cxx_std_20)
+if(TRIESTE_USE_CXX17)
+  target_compile_definitions(trieste INTERFACE cxx_std_17)
+else()
+  target_compile_definitions(trieste INTERFACE cxx_std_20)
+endif()
 
 if(MSVC)
   target_compile_options(trieste INTERFACE /W4 /WX /wd5030 /bigobj)

include/trieste/pass.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "rewrite.h"
+
 #include <vector>
 
 namespace trieste
@@ -11,7 +12,7 @@ namespace trieste
     constexpr flag bottomup = 1 << 0;
     constexpr flag topdown = 1 << 1;
     constexpr flag once = 1 << 2;
-  };
+  }
 
   class PassDef;
   using Pass = std::shared_ptr<PassDef>;
@@ -234,7 +235,7 @@ namespace trieste
       add(root);
       while (!path.empty())
       {
-        auto& [node,it] = path.back();
+        auto& [node, it] = path.back();
         if (it != node->end())
         {
           Node curr = *it;

include/trieste/source.h

@@ -41,7 +41,7 @@ namespace trieste
 
       auto source = std::make_shared<SourceDef>();
       source->origin_ = file.string();
-      source->contents.resize(size);
+      source->contents.resize(static_cast<std::size_t>(size));
       f.read(&source->contents[0], size);
 
       if (!f)

include/trieste/wf.h

@@ -81,10 +81,26 @@ namespace trieste
           tokens.end(),
           std::back_inserter(offsets),
           [&](const Token& t) {
-            return 1.0 /
-              (1.0 +
-               (alpha * (depth - target_depth) *
-                token_terminal_distance.at(t)));
+
+            if (token_terminal_distance.find(t) != token_terminal_distance.end())
+            {
+              std::size_t distance = token_terminal_distance.at(t);
+              return 1.0 /
+                (1.0 +
+                (alpha * (depth - target_depth) *
+                  distance));
+            }else{
+              std::ostringstream err;
+              err << "Token " << t.str() << " not found in token_terminal_distance map" << std::endl;
+              err << "{";
+              std::string delim = "";
+              for(auto const& [key, val] : token_terminal_distance){
+                err << delim << key.str() << ":" << val;
+                delim = ", ";
+              }
+              err << "}" << std::endl;
+              throw std::runtime_error(err.str());
+            }
           });
 
         // compute the cumulative distribution of P(d | c, p)
@@ -165,7 +181,7 @@ namespace trieste
                  types.end(),
                  static_cast<std::size_t>(0),
                  [&](std::size_t acc, auto& type) {
-                   if (omit.contains(type))
+                   if (omit.find(type) != omit.end())
                    {
                      return acc + max_distance;
                    }
@@ -523,12 +539,12 @@ namespace trieste
         std::size_t max_distance,
         const Token& token) const
       {
-        if (distance.contains(token))
+        if (distance.find(token) != distance.end())
         {
           return distance[token];
         }
 
-        if (!shapes.contains(token))
+        if (shapes.find(token) == shapes.end())
         {
           distance[token] = 0;
         }