Resolve compiler warnings and upgrade to C++17

Makefile

@@ -227,8 +227,9 @@ else
     $(info OS is Linux)
     $(info Compiler $(CXX) is assumed to be GCC)
 
-    # Linux can have some old compilers so we want to work back to C++14
-    CXX_STANDARD?=14
+	# gbwtgraph uses inline variables and our oldest supported compiler has
+	# C++17, so we should use C++17
+    CXX_STANDARD?=17
 
     # Set an rpath for vg and dependency utils to find installed libraries
     LD_UTIL_RPATH_FLAGS="-Wl,-rpath,$(CWD)/$(LIB_DIR)"

README.md

@@ -93,7 +93,7 @@ On other distros, or if you do not have root access, you will need to perform th
                          liblzma-dev liblz4-dev libffi-dev libcairo-dev libboost-all-dev \
                          libzstd-dev pybind11-dev python3-pybind11 libssl-dev
 
-At present, you will need GCC version 9 or greater, with support for C++14, to compile vg. (Check your version with `gcc --version`.) GCC up to 11.4.0 is supported.
+At present, you will need GCC version 9 or greater, with support for C++17, to compile vg. (Check your version with `gcc --version`.) GCC up to 11.4.0 is supported.
 
 Other libraries may be required. Please report any build difficulties.
 

src/gaf_sorter.cpp

@@ -5,6 +5,7 @@
 #include <fstream>
 #include <queue>
 #include <thread>
+#include <charconv>
 
 // Needed for the temporary file creation.
 #include "utility.hpp"
@@ -19,45 +20,6 @@
 // For building a GBWT index of the paths.
 #include <gbwt/dynamic_gbwt.h>
 
-#if  __cplusplus >= 201703L
-
-// On C++17, we have std::from_chars
-#include <charconv>
-using std::from_chars;
-
-#else
-
-// Before C++17, we polyfill it.
-// Some compilers might still expose <charconv>, but we don't try and use it.
-struct from_chars_result {
-    const char* ptr;
-    std::errc ec;
-};
-
-template<typename IntVal>
-from_chars_result from_chars(const char* begin, const char* end, IntVal& dest) {
-    static_assert(std::is_integral<IntVal>::value, "Polyfill can only parse integers");
-    static_assert(std::is_unsigned<IntVal>::value, "Polyfill can only parse usigned integers");
-    const char* here = begin;
-    IntVal result = 0;
-    while (here != end && *here >= '0' && *here <= '9') {
-        // Collect all the digits
-        result *= (IntVal)10;
-        result += (IntVal)(*here - '0');
-        ++here;
-    }
-    if (here == begin) {
-        // No number present.
-        return {here, std::errc::invalid_argument};
-    }
-    // Otherwise we parsed something.
-    dest = result;
-    return {here, std::errc()};
-}
-
-#endif
-
-
 namespace vg {
 
 //------------------------------------------------------------------------------
@@ -85,7 +47,7 @@ void GAFSorterRecord::set_key(key_type type) {
         size_t start = 1;
         while (start < path.size) {
             std::uint32_t id = 0;
-            auto result = from_chars(path.data + start, path.data + path.size, id);
+            auto result = std::from_chars(path.data + start, path.data + path.size, id);
             if (result.ec != std::errc()) {
                 this->key = MISSING_KEY;
                 return;
@@ -205,7 +167,7 @@ gbwt::vector_type GAFSorterRecord::as_gbwt_path(bool* ok) const {
         }
         start++;
         gbwt::size_type node_id = 0;
-        auto res = from_chars(path.data + start, path.data + path.size, node_id);
+        auto res = std::from_chars(path.data + start, path.data + path.size, node_id);
         if (res.ec != std::errc() || node_id == 0) {
             if (ok != nullptr) {
                 *ok = false;

src/multipath_alignment.cpp

@@ -816,16 +816,9 @@ namespace vg {
         // Run the dynamic programming
         auto dp_result = run_multipath_dp(multipath_aln, subpath_global);
 
-        // C++17 finally gets http://en.cppreference.com/w/cpp/language/structured_binding
-        // Until then we have to unpack tuples like this.
-
-        // Get the filled DP problem
-        MultipathProblem& problem = get<0>(dp_result);
-        // And the optimal final subpath
-        int64_t& opt_subpath = get<1>(dp_result);
-        // And the optimal score
-        int32_t& opt_score = get<2>(dp_result);
-
+        // Get the filled DP problem, the optimal final subpath, and the optimal score
+        auto& [problem, opt_subpath, opt_score] = dp_result;
+
         // are we constructing the alignment, or just getting the score?
         if (aln_out && opt_subpath >= 0) {
 

src/subcommand/options.hpp

@@ -913,14 +913,9 @@ struct OptionGroup : public BaseOptionGroup {
         return false;
     }
 
-    // We need to take default_value by value, and not by reference, because we
-    // often want to pass stuff that is constexpr and trying to use a reference
-    // will make us try to link against it.
-    // TODO: C++17 fixes this, so fix when we use that.
-
     /// Add a new option that goes to the given field, with the given default.
     template<typename T, typename Spec = ValueArgSpec<T, Receiver>>
-    void add_option(const std::string& name, char short_option, T Receiver::*dest, T default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
+    void add_option(const std::string& name, char short_option, T Receiver::*dest, const T& default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
         args.emplace_back(new Spec(name, short_option, dest, default_value, help, validator));
         if (args.size() == 1) {
             // Chain us to first arg
@@ -937,18 +932,18 @@ struct OptionGroup : public BaseOptionGroup {
 
     /// Add a new option that goes to the given field, with the given default.
     template<typename T, typename Spec = ValueArgSpec<T, Receiver>>
-    void add_option(const std::string& name, T Receiver::*dest, T default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
+    void add_option(const std::string& name, T Receiver::*dest, const T& default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
         add_option<T, Spec>(name, '\0', dest, default_value, help, validator);
     }
 
     /// Add a new option that handles range values
     template<typename T>
-    void add_range(const std::string& name, char short_option, T Receiver::*dest, T default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
+    void add_range(const std::string& name, char short_option, T Receiver::*dest, const T& default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
         add_option<T, RangeArgSpec<T, Receiver>>(name, short_option, dest, default_value, help, validator);
     }
     /// Add a new option that handles range values
     template<typename T>
-    void add_range(const std::string& name, T Receiver::*dest, T default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
+    void add_range(const std::string& name, T Receiver::*dest, const T& default_value, const std::string& help, const ValidatorFunction<T>& validator = [](const T& ignored) {}) {
         add_range<T>(name, '\0', dest, default_value, help, validator);
     }
 

src/subcommand/sim_main.cpp

@@ -652,12 +652,12 @@ int main_sim(int argc, char** argv) {
                 path_ploidies.push_back(1.0);
                 ++sample_path_count;
             });
-
+            
             if (seen_sample_names.size() != sample_name_set.size()) {
-                // TODO: Use std::set_difference in C++17
                 auto error_msg = logger.error();
                 error_msg << "Some samples requested are not in the graph:";
                 for (auto& s : sample_name_set) {
+                    // The STL doesn't have a widget to subtract sets as of C++17.
                     if (!seen_sample_names.count(s)) {
                         error_msg << " " << s;
                     }

src/surjector.cpp

@@ -432,6 +432,24 @@ using namespace std;
                 transfer_read_metadata(*source_mp_aln, mp_alns_out->back());
             }
             else {
+                // We already constrained source_aln and alns_out to both be set if source_mp_aln is unset.
+                // But g++ 11.4 will warn that it thinks alns_out can be null here.
+                // Advise it that it can't.
+                // TODO: There's not a good way to actually silence a
+                // particular warning; we want to convince the compiler this
+                // can't happen without actually generating any code.
+#if __cplusplus >= 202302L
+                [[assume(alns_out)]];
+#elif __GNUC__ >= 13
+                __attribute__((__assume__(alns_out)));
+#elif defined(__clang__)
+                __builtin_assume(alns_out);
+#else
+                if (!alns_out) {
+                    __builtin_unreachable();
+                }
+#endif
+
                 alns_out->emplace_back(make_null_alignment(*source_aln));
             }
             return;

src/utility.cpp

@@ -7,6 +7,7 @@
 #include <thread>
 #include <cstdio>
 #include <cmath>
+#include <filesystem>
 #include <fstream>
 #include <iostream>
 #include <cctype>
@@ -705,14 +706,17 @@ string file_base_name(const string& filename) {
 }
 
 bool file_exists(const string& filename) {
-    // TODO: use C++17 features to actually poll existence.
-    // For now we see if we can open it.
     if (filename == "-") {
-        // Standard input is always open
+        // Standard input should be thought of as existing.
         return true;
     }
-    ifstream in(filename);
-    return in.is_open();
+    try {
+        return std::filesystem::exists(filename);
+    } catch (const std::filesystem::filesystem_error& e) {
+        logging::warn("file_exists") << "Unable to determine if " << filename << " exists: " << e.what() << std::endl;  
+        // If we can't tell it exists, it doesn't exist.
+        return false;
+    }
 }
 
 bool file_can_be_written(const string& filename) {