Update standards to c++17, Qt >= 5.12, MSVC >= 2017 (GPSBabel#670)

* raise minimum standards to c++17, Qt 5.12, MSVC 2017.

* avoid std::optional::value which didn't work until macOS 10.14.

* avoid std::optional::value which didn't work until macOS 10.14.

* update runner for codeql so we have Qt >= 5.12

* try harder to get codeql working with newer Qt.

* use c++17 fallthrough attribute.

* use c++17 'if constexpr' as suggested by resharper.

* catch mkicondoc up with c++17

.github/workflows/codeql-analysis.yml

@@ -25,7 +25,7 @@ jobs:
   analyze:
     name: Analyze
     runs-on: ubuntu-latest
-    container: tsteven4/gpsbabel_build_environment
+    container: tsteven4/gpsbabel_build_environment_focal
 
     strategy:
       fail-fast: false

.github/workflows/ubuntu.yml

@@ -14,7 +14,7 @@ jobs:
     name: basic Build
     runs-on: ubuntu-latest
     container:
-      image: gpsbabel-docker.jfrog.io/tsteven4/gpsbabel_build_environment
+      image: gpsbabel-docker.jfrog.io/tsteven4/gpsbabel_build_environment_focal
       env:
         LC_ALL: 'C.UTF-8'
 

.github/workflows/windows.yml

@@ -43,13 +43,6 @@ jobs:
             VCVERSION: '14.16'
             FLOW: 'nmake'
             RELEASE: false
-          - QT_VERSION: '5.12.10'
-            ARCH: 'amd64'
-            HOST_ARCH: 'x86'
-            COMPILER: 'msvc2017_64'
-            VCVERSION: '14.0'
-            FLOW: 'nmake'
-            RELEASE: false
           - QT_VERSION: '5.12.10'
             ARCH: 'x86'
             HOST_ARCH: 'x86'

CMakeLists.txt

@@ -7,15 +7,15 @@ cmake_minimum_required(VERSION 3.5.1)
 
 project(gpsbabel LANGUAGES C CXX)
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED True)
 
 # Find includes in corresponding build directories
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
 
 # Find the Qt5Core library
 find_package(Qt5 COMPONENTS Core REQUIRED)
-if(${Qt5Core_VERSION} VERSION_LESS 5.9)
+if(${Qt5Core_VERSION} VERSION_LESS 5.12)
   message(FATAL_ERROR "Qt version ${Qt5Core_VERSION} found, but version 5.9 or newer is required.")
 else()
   message(STATUS "Using Qt5 version ${Qt5Core_VERSION}")
@@ -179,7 +179,6 @@ set(HEADERS
   src/core/datetime.h
   src/core/file.h
   src/core/logging.h
-  src/core/optional.h
   src/core/textstream.h
   src/core/usasciicodec.h
   src/core/xmlstreamwriter.h

GPSBabel.pro

@@ -1,6 +1,6 @@
 # Enforce minimum Qt version.
 # versionAtLeast() was introduced in Qt 5.10, so we can't count on it being available.
-MIN_QT_VERSION = 5.9 # major[.minor[.patch]]
+MIN_QT_VERSION = 5.12 # major[.minor[.patch]]
 MIN_QT_VERSION_COMPONENTS = $$split(MIN_QT_VERSION, .)
 MIN_QT_VERSION_MAJOR = $$member(MIN_QT_VERSION_COMPONENTS, 0)
 MIN_QT_VERSION_MINOR = $$member(MIN_QT_VERSION_COMPONENTS, 1)
@@ -20,7 +20,7 @@ VERSION = 1.7.0
 
 CONFIG += console
 CONFIG -= app_bundle
-CONFIG += c++14
+CONFIG += c++17
 CONFIG += link_pkgconfig
 
 TEMPLATE = app
@@ -182,7 +182,6 @@ HEADERS =  \
 	src/core/datetime.h \
 	src/core/file.h \
 	src/core/logging.h \
-	src/core/optional.h \
 	src/core/textstream.h \
 	src/core/usasciicodec.h \
 	src/core/xmlstreamwriter.h \

defs.h

@@ -26,6 +26,7 @@
 #include <cstdint>                // for int32_t, uint32_t
 #include <cstdio>                 // for NULL, fprintf, FILE, stdout
 #include <ctime>                  // for time_t
+#include <optional>               // for optional
 #include <utility>                // for move
 
 #if HAVE_CONFIG_H
@@ -52,7 +53,6 @@
 #include "gbfile.h"               // doesn't really belong here, but is missing elsewhere.
 #include "session.h"              // for session_t
 #include "src/core/datetime.h"    // for DateTime
-#include "src/core/optional.h"    // for optional
 
 
 #define CSTR(qstr) ((qstr).toUtf8().constData())
@@ -676,17 +676,17 @@ waypt_disp_all(T cb)
  */
 struct computed_trkdata {
   double distance_meters{0.0};
-  gpsbabel_optional::optional<double> max_alt;	/* Meters */
-  gpsbabel_optional::optional<double> min_alt;	/* Meters */
-  gpsbabel_optional::optional<double> max_spd;	/* Meters/sec */
-  gpsbabel_optional::optional<double> min_spd;	/* Meters/sec */
-  gpsbabel_optional::optional<double> avg_hrt;	/* Avg Heartrate */
-  gpsbabel_optional::optional<double> avg_cad;	/* Avg Cadence */
+  std::optional<double> max_alt;	/* Meters */
+  std::optional<double> min_alt;	/* Meters */
+  std::optional<double> max_spd;	/* Meters/sec */
+  std::optional<double> min_spd;	/* Meters/sec */
+  std::optional<double> avg_hrt;	/* Avg Heartrate */
+  std::optional<double> avg_cad;	/* Avg Cadence */
   gpsbabel::DateTime start;		/* Min time */
   gpsbabel::DateTime end;		/* Max time */
-  gpsbabel_optional::optional<int> min_hrt;			/* Min Heartrate */
-  gpsbabel_optional::optional<int> max_hrt;			/* Max Heartrate */
-  gpsbabel_optional::optional<int> max_cad;			/* Max Cadence */
+  std::optional<int> min_hrt;			/* Min Heartrate */
+  std::optional<int> max_hrt;			/* Max Heartrate */
+  std::optional<int> max_cad;			/* Max Cadence */
 };
 
 class route_head
@@ -974,20 +974,6 @@ void setshort_defname(short_handle, const char* s);
 #define ARG_NOMINMAX nullptr, nullptr
 
 struct arglist_t {
-  /* MSVC 2015 generates C2440, C2664 errors without some help. */
-#if defined(_MSC_VER) && (_MSC_VER < 1910) /* MSVC 2015 or earlier */
-  arglist_t() = default;
-  arglist_t(const char* astr, char** aval, const char* hstr, const char* dval,
-            const uint32_t atyp, const char* minv, const char* maxv, char* avp) :
-            argstring(astr),
-            argval(aval),
-            helpstring(hstr),
-            defaultvalue(dval),
-            argtype(atyp),
-            minvalue(minv),
-            maxvalue(maxv),
-            argvalptr(avp) {}
-#endif
   const char* argstring{nullptr};
   char** argval{nullptr};
   const char* helpstring{nullptr};

delgpl.cc

@@ -42,7 +42,7 @@ static void
 gpl_rd_init(const QString& fname)
 {
   gplfile_in = gbfopen_le(fname, "rb", MYNAME);
-  if (sizeof(gpl_point_t) != 56) {
+  if constexpr (sizeof(gpl_point_t) != 56) {
     fatal(MYNAME ": gpl_point is %zu instead of 56.\n",
           sizeof(gpl_point_t));
   }

dmtlog.cc

@@ -585,7 +585,7 @@ inflate_buff(const char* buff, const size_t size, char** out_buff)
     switch (res) {
     case Z_NEED_DICT:
       res = Z_DATA_ERROR;
-      /* fallthrough */
+      [[fallthrough]];
     case Z_DATA_ERROR:
     case Z_MEM_ERROR:
       (void)inflateEnd(&strm);

g7towin.cc

@@ -188,7 +188,7 @@ parse_line(char* buff, int index, const char* delimiter, Waypoint* wpt)
       if (strcmp(cin, "1.0e25") == 0) {
         break;
       }
-      /* fallthrough */
+      [[fallthrough]];
     case WPT_cD_OFS + 1:
     case WPT_cB_OFS + 6:
       WAYPT_SET(wpt, temperature, atof(cin));

garmin_fit.cc

@@ -47,18 +47,6 @@
 
 #define MYNAME "fit"
 
-// Until c++17 we have to define odr-used constexpr static data members at namespace scope.
-#if __cplusplus < 201703L
-constexpr int GarminFitFormat::kTypeEnum;
-constexpr int GarminFitFormat::kTypeUint8;
-constexpr int GarminFitFormat::kTypeString;
-constexpr int GarminFitFormat::kTypeUint16;
-constexpr int GarminFitFormat::kTypeSint32;
-constexpr int GarminFitFormat::kTypeUint32;
-constexpr int GarminFitFormat::kCoursePointTypeLeft;
-constexpr int GarminFitFormat::kCoursePointTypeRight;
-#endif
-
 /*******************************************************************************
 * %%%        global callbacks called by gpsbabel main process              %%% *
 *******************************************************************************/

garmin_fit.h

@@ -84,11 +84,6 @@ class GarminFitFormat : public Format
   /* Types */
 
   struct fit_field_t {
-    /* MSVC 2015 generates C2664 errors without some help. */
-#if defined(_MSC_VER) && (_MSC_VER < 1910) /* MSVC 2015 or earlier */
-    fit_field_t() = default;
-    fit_field_t(int i, int s, int t) : id(i), size(s), type(t) {}
-#endif
     int id {};
     int size{};
     int type{};

gpx.h

@@ -182,11 +182,6 @@ class GpxFormat : public Format
   };
 
   struct tag_mapping {
-#if defined(_MSC_VER) && (_MSC_VER < 1910) /* MSVC 2015 or earlier */
-    /* avoid MSVC 2015 C2664 errors. */
-    tag_mapping() = default;
-    tag_mapping(tag_type t, bool p) : type(t),passthrough(p) {}
-#endif
     tag_type type{tt_unknown};		/* enum from above for this tag */
     bool passthrough{true};		/* true if we don't generate this */
   };

gui/CMakeLists.txt

@@ -7,7 +7,7 @@ cmake_minimum_required(VERSION 3.5.1)
 
 project(gpsbabelfe LANGUAGES CXX)
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED True)
 
 # Find includes in corresponding build directories
@@ -22,7 +22,7 @@ set(CMAKE_AUTORCC ON)
 # Find the Qt5Core library
 find_package(Qt5 COMPONENTS Core Gui Network Widgets Xml REQUIRED)
 list(APPEND QT_LIBRARIES Qt5::Core Qt5::Gui Qt5::Network Qt5::Widgets Qt5::Xml)
-if(${Qt5Core_VERSION} VERSION_LESS 5.9)
+if(${Qt5Core_VERSION} VERSION_LESS 5.12)
   message(FATAL_ERROR "Qt version ${Qt5Core_VERSION} found, but version 5.9 or newer is required.")
 else()
   message(STATUS "Using Qt5 version ${Qt5Core_VERSION}")

gui/app.pro

@@ -5,6 +5,7 @@ CONFIG += qt
 CONFIG(debug, debug|release) {
   CONFIG += console
 }
+CONFIG += c++17
 
 ICON = images/appicon.icns
 

gui/main.cc

@@ -29,8 +29,10 @@
 //------------------------------------------------------------------------
 int main(int argc, char** argv)
 {
-// MIN_QT_VERSION in configure.ac should correspond to the QT_VERSION_CHECK arguments in main.cc and gui/main.cc
-#if (QT_VERSION < QT_VERSION_CHECK(5, 9, 0))
+// MIN_QT_VERSION in GPSBabel.pro should correspond to the QT_VERSION_CHECK
+// arguments in main.cc and gui/main.cc and the version check in
+// CMakeLists.txt, gui/CMakeLists.txt.
+#if (QT_VERSION < QT_VERSION_CHECK(5, 12, 0))
 #error this version of Qt is not supported.
 #endif
 

height.cc

@@ -32,15 +32,6 @@
 
 #if FILTERS_ENABLED
 
-// Until c++17 we have to define odr-used constexpr static data members at namespace scope.
-#if __cplusplus < 201703L
-constexpr double HeightFilter::geoid_grid_deg;
-constexpr double HeightFilter::geoid_scale;
-constexpr int HeightFilter::geoid_row;
-constexpr int HeightFilter::geoid_col;
-constexpr int8_t HeightFilter::geoid_delta[geoid_row][geoid_col];
-#endif
-
 double HeightFilter::bilinear(double x1, double y1, double x2, double y2, double x, double y, double z11, double z12, double z21, double z22)
 {
   if (y1 == y2 && x1 == x2) {

igc.cc

@@ -29,6 +29,7 @@
 #include <cstring>                   // for strcmp, strlen, strtok, strcat, strchr, strcpy, strncat
 #include <ctime>                     // for gmtime, ctime
 #include <iterator>                  // for reverse_iterator, operator==, prev, next
+#include <optional>                  // for optional
 
 #include <QtCore/QByteArray>         // for QByteArray
 #include <QtCore/QList>              // for QList<>::const_iterator
@@ -40,7 +41,6 @@
 #include "cet_util.h"                // for cet_convert_init
 #include "gbfile.h"                  // for gbfprintf, gbfclose, gbfopen, gbfputs, gbfgetstr, gbfile
 #include "src/core/datetime.h"       // for DateTime
-#include "src/core/optional.h"       // for optional
 
 
 static gbfile* file_in, *file_out;
@@ -815,8 +815,8 @@ static int correlate_tracks(const route_head* pres_track, const route_head* gnss
  */
 static double interpolate_alt(const route_head* track, time_t time)
 {
-  static gpsbabel_optional::optional<WaypointList::const_iterator> prev_wpt;
-  static gpsbabel_optional::optional<WaypointList::const_iterator> curr_wpt;
+  static std::optional<WaypointList::const_iterator> prev_wpt;
+  static std::optional<WaypointList::const_iterator> curr_wpt;
   int time_diff;
 
   // Start search at the beginning of the track
@@ -825,32 +825,32 @@ static double interpolate_alt(const route_head* track, time_t time)
     curr_wpt = track->waypoint_list.cbegin();
   }
   // Find the track points either side of the requested time
-  while ((track->waypoint_list.cend() != curr_wpt.value()) &&
-         ((*curr_wpt.value())->GetCreationTime().toTime_t() < time)) {
-    prev_wpt = curr_wpt.value();
-    curr_wpt = std::next(prev_wpt.value());
+  while ((track->waypoint_list.cend() != *curr_wpt) &&
+         ((**curr_wpt)->GetCreationTime().toTime_t() < time)) {
+    prev_wpt = *curr_wpt;
+    curr_wpt = std::next(*prev_wpt);
   }
-  if (track->waypoint_list.cend() == curr_wpt.value()) {
+  if (track->waypoint_list.cend() == *curr_wpt) {
     // Requested time later than all track points, we can't interpolate
     return unknown_alt;
   }
 
-  if (track->waypoint_list.cbegin() == curr_wpt.value()) {
-    if ((*curr_wpt.value())->GetCreationTime().toTime_t() == time) {
+  if (track->waypoint_list.cbegin() == *curr_wpt) {
+    if ((**curr_wpt)->GetCreationTime().toTime_t() == time) {
       // First point's creation time is an exact match so use it's altitude
-      return (*curr_wpt.value())->altitude;
+      return (**curr_wpt)->altitude;
     } else {
       // Requested time is prior to any track points, we can't interpolate
       return unknown_alt;
     }
   }
   // Interpolate
-  if (0 == (time_diff = (*curr_wpt.value())->GetCreationTime().toTime_t() - (*prev_wpt.value())->GetCreationTime().toTime_t())) {
+  if (0 == (time_diff = (**curr_wpt)->GetCreationTime().toTime_t() - (**prev_wpt)->GetCreationTime().toTime_t())) {
     // Avoid divide by zero
-    return (*curr_wpt.value())->altitude;
+    return (**curr_wpt)->altitude;
   }
-  double alt_diff = (*curr_wpt.value())->altitude - (*prev_wpt.value())->altitude;
-  return (*prev_wpt.value())->altitude + (alt_diff / time_diff) * (time - (*prev_wpt.value())->GetCreationTime().toTime_t());
+  double alt_diff = (**curr_wpt)->altitude - (**prev_wpt)->altitude;
+  return (**prev_wpt)->altitude + (alt_diff / time_diff) * (time - (**prev_wpt)->GetCreationTime().toTime_t());
 }
 
 /*

igo8.cc

@@ -128,17 +128,17 @@ static QVector<arglist_t> igo8_options = {
 // Sanity check
 static void igo8_check_type_sizes()
 {
-  if (sizeof(igo8_point) != 12) {
+  if constexpr (sizeof(igo8_point) != 12) {
     fatal(MYNAME ": igo8_point is %ld bytes instead of the required 12.\n",
           (long) sizeof(igo8_point));
   }
 
-  if (sizeof(igo8_information_block) != 12) {
+  if constexpr (sizeof(igo8_information_block) != 12) {
     fatal(MYNAME ": igo8_information_block is %ld bytes instead of the required 12.\n",
           (long) sizeof(igo8_information_block));
   }
 
-  if (sizeof(igo8_id_block) != 20) {
+  if constexpr (sizeof(igo8_id_block) != 20) {
     fatal(MYNAME ": igo8_id_block is %ld bytes instead of the required 20.\n",
           (long) sizeof(igo8_id_block));
   }