From ebe532bd5f6fec8cad4a524e38f6df84f4e1c9f3 Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Wed, 26 Aug 2026 10:09:47 -0400 Subject: [PATCH 1/4] Always build using C++17 We were using C++14 in the cmake build and only conditionally using it in the autotools build, and we'll need it to use constexpr for implicit inlining. --- CMakeLists.txt | 2 +- configure.ac | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1cb4a8f..6a60fe0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.12) -set(CMAKE_CXX_STANDARD 14) +set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) project(mathic VERSION 1.4 LANGUAGES CXX) diff --git a/configure.ac b/configure.ac index 8eca3f4..58bc3e1 100644 --- a/configure.ac +++ b/configure.ac @@ -70,6 +70,8 @@ dnl source and binary interface of the library and is not the same as the dnl version of the project. AC_SUBST([MATHIC_SO_VERSION], [0:4:0]) +AX_CXX_COMPILE_STDCXX([17],,[mandatory]) + dnl ----- The gtest dependency AC_ARG_WITH([gtest], @@ -83,7 +85,6 @@ AC_DEFUN([NO_GTEST_ERROR], AS_IF([test "x$with_gtest" != "xno"], [AC_LANG([C++]) - AX_CXX_COMPILE_STDCXX([17],,[mandatory]) AC_CHECK_HEADER([gtest/gtest.h], [AC_MSG_CHECKING([for library containing testing::InitGoogleTest]) SAVELIBS=$LIBS From 2e646010504656f9ddf3d6d10cbc5d4a29407a08 Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Wed, 26 Aug 2026 10:17:07 -0400 Subject: [PATCH 2/4] Use constexpr for static const data members A static const data member with an in-class initializer is only a declaration, not a definition. C++ requires a separate out-of-line definition at namespace scope if such a member is ever odr-used, and we never provided one anywhere. Merely reading a member's value kept working, since the compiler constant-folds it and emits no symbol at all, but any odr-use -- binding a reference to it, or taking its address -- had no definition to resolve against and failed to link. Nothing odr-used these members, so this went unnoticed. googletest 1.18.0 became the first odr-use, of PairQueue::SupportRetirement. Through 1.17, ASSERT_FALSE(x) expanded to AssertionResult(!(x)); the negation yielded a temporary for AssertionResult's const T& parameter to bind to, so the member's own address was never needed. In 1.18 the negation moved into the new AssertionResultExpectation type and the condition is passed through verbatim, so that reference now binds directly to the member: /usr/bin/x86_64-linux-gnu-ld.bfd: PairQueue.o: in function `PairQueue_RetirementSetToFalse_Test::TestBody()': PairQueue.cpp:(.text+0x1bf): undefined reference to `mathic::PairQueue<(anonymous namespace)::PQConf>::SupportRetirement' Since we now always compile with C++17, where constexpr static data members are implicitly inline, the in-class declaration is itself a definition and no out-of-line definitions are needed. Convert all 28 affected members rather than only the one googletest happened to expose, as the others are latent failures for any caller that odr-uses them. This includes the divsim and pqsim configurations, which are easy to overlook because both programs are opt-in and a default build never compiles them. Namespace-scope statics are left alone: those have internal linkage and are already defined in every translation unit, so they are not affected. Configurations supplied by users need not change, as a static const bool with a constant initializer remains usable in these constexpr initializers. Say so explicitly in the DivFinder.h documentation of the UseDivMask requirement, which previously called for a static const data member specifically. Co-Authored-By: Claude Opus 5 --- src/divsim/DivListModel.h | 4 ++-- src/divsim/KDTreeModel.h | 10 +++++----- src/mathic/BinaryKDTree.h | 2 +- src/mathic/ComTree.h | 4 ++-- src/mathic/DivFinder.h | 5 +++-- src/mathic/DivList.h | 4 ++-- src/mathic/KDTree.h | 2 +- src/mathic/PackedKDTree.h | 2 +- src/mathic/PairQueue.h | 7 +++---- src/pqsim/GeobucketModel.h | 12 ++++++------ src/pqsim/HeapModel.h | 2 +- src/pqsim/Model.h | 2 +- src/pqsim/TourTreeModel.h | 2 +- src/test/PairQueue.cpp | 4 ++-- 14 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/divsim/DivListModel.h b/src/divsim/DivListModel.h index eda34db..c8ce2cd 100644 --- a/src/divsim/DivListModel.h +++ b/src/divsim/DivListModel.h @@ -29,8 +29,8 @@ class DivListModelConfiguration { _minRebuild(minRebuild), _expQueryCount(0) {} - static const bool UseLinkedList = ULL; - static const bool UseDivMask = UDM; + static constexpr bool UseLinkedList = ULL; + static constexpr bool UseDivMask = UDM; bool getDoAutomaticRebuilds() const {return _useAutomaticRebuild;} double getRebuildRatio() const {return _rebuildRatio;} diff --git a/src/divsim/KDTreeModel.h b/src/divsim/KDTreeModel.h index 7b5a44f..a001983 100644 --- a/src/divsim/KDTreeModel.h +++ b/src/divsim/KDTreeModel.h @@ -70,11 +70,11 @@ class KDTreeModelConfiguration { double getRebuildRatio() const {return _rebuildRatio;} size_t getRebuildMin() const {return _minRebuild;} - static const bool UseDivMask = UDM; - static const bool UseTreeDivMask = UTDM; - static const bool PackedTree = PT; - static const size_t LeafSize = LS; - static const bool AllowRemovals = AR; + static constexpr bool UseDivMask = UDM; + static constexpr bool UseTreeDivMask = UTDM; + static constexpr bool PackedTree = PT; + static constexpr size_t LeafSize = LS; + static constexpr bool AllowRemovals = AR; unsigned long long getExpQueryCount() const {return _expQueryCount;} diff --git a/src/mathic/BinaryKDTree.h b/src/mathic/BinaryKDTree.h index f3b24b9..8fb7cd6 100644 --- a/src/mathic/BinaryKDTree.h +++ b/src/mathic/BinaryKDTree.h @@ -46,7 +46,7 @@ namespace mathic { typedef KDTreeInterior Interior; typedef KDTreeLeaf Leaf; typedef C Configuration; - static const bool UseDivMask = C::UseDivMask; + static constexpr bool UseDivMask = C::UseDivMask; class KDTreeNode { public: diff --git a/src/mathic/ComTree.h b/src/mathic/ComTree.h index 12896b7..75e5f58 100644 --- a/src/mathic/ComTree.h +++ b/src/mathic/ComTree.h @@ -88,8 +88,8 @@ namespace mathic { //private: friend class ComTree; - static const bool fi = FastIndex; - static const size_t S = sizeof(Entry); + static constexpr bool fi = FastIndex; + static constexpr size_t S = sizeof(Entry); explicit Node(size_t i): _index(i) {} size_t getNormalIndex() const {return fi ? _index / S : _index;} size_t _index; diff --git a/src/mathic/DivFinder.h b/src/mathic/DivFinder.h index 7fbb933..8e40801 100644 --- a/src/mathic/DivFinder.h +++ b/src/mathic/DivFinder.h @@ -38,9 +38,10 @@ Returns the number of variables. Variables are indexed from 0 to getVarCount() - 1. Must be a positive number. - * static const bool UseDivMask + * static constexpr bool UseDivMask Set to true to use div masks to speed up queries. This must be a - static const data member. + compile-time constant data member (static constexpr bool, or static + const bool with a constant initializer). * size_t getUseAutomaticRebuild() const * double getRebuildRatio() const diff --git a/src/mathic/DivList.h b/src/mathic/DivList.h index 780d14c..f2b8241 100644 --- a/src/mathic/DivList.h +++ b/src/mathic/DivList.h @@ -53,8 +53,8 @@ namespace mathic { typedef typename C::Monomial Monomial; typedef typename C::Exponent Exponent; - static const bool UseLinkedList = C::UseLinkedList; - static const bool UseDivMask = C::UseDivMask; + static constexpr bool UseLinkedList = C::UseLinkedList; + static constexpr bool UseDivMask = C::UseDivMask; private: typedef typename DivMask::Extender ExtEntry; diff --git a/src/mathic/KDTree.h b/src/mathic/KDTree.h index 8290bb2..95dbb66 100644 --- a/src/mathic/KDTree.h +++ b/src/mathic/KDTree.h @@ -68,7 +68,7 @@ namespace mathic { _divisorCache = 0; } - static const bool UseDivMask = C::UseDivMask; + static constexpr bool UseDivMask = C::UseDivMask; typedef typename C::Monomial Monomial; typedef typename C::Entry Entry; typedef typename C::Exponent Exponent; diff --git a/src/mathic/PackedKDTree.h b/src/mathic/PackedKDTree.h index 19dd091..f007c7b 100644 --- a/src/mathic/PackedKDTree.h +++ b/src/mathic/PackedKDTree.h @@ -30,7 +30,7 @@ namespace mathic { private: typedef C Configuration; - static const bool UseDivMask = C::UseDivMask; + static constexpr bool UseDivMask = C::UseDivMask; class Node { public: diff --git a/src/mathic/PairQueue.h b/src/mathic/PairQueue.h index 273dcd3..7071b52 100644 --- a/src/mathic/PairQueue.h +++ b/src/mathic/PairQueue.h @@ -131,8 +131,7 @@ namespace mathic { // configuration. template struct SupportRetirement { - // the value field must be static bool const. - static bool const value = true; + static constexpr bool value = true; }; } @@ -198,7 +197,7 @@ namespace mathic { typedef C Configuration; typedef typename C::PairData PairData; typedef PairQueueNamespace::Index Index; - static bool const SupportRetirement = + static constexpr bool SupportRetirement = PairQueueNamespace::SupportRetirement::value; // PairQueue stores a copy of the passed in configuration. @@ -365,7 +364,7 @@ namespace mathic { return mConf.cmpLessThan(cr); } - static const bool fastIndex = false; + static constexpr bool fastIndex = false; private: Configuration& mConf; diff --git a/src/pqsim/GeobucketModel.h b/src/pqsim/GeobucketModel.h index fef45f7..27ff6e6 100644 --- a/src/pqsim/GeobucketModel.h +++ b/src/pqsim/GeobucketModel.h @@ -18,13 +18,13 @@ class GeobucketModelBase { size_t geoBase; size_t minBucketSize; - static const bool trackFront = TrackFront; - static const bool minBucketBinarySearch = MinBucketBinarySearch; - static const bool premerge = Premerge; - static const bool collectMax = CollectMax; - static const mathic::GeobucketBucketStorage bucketStorage = + static constexpr bool trackFront = TrackFront; + static constexpr bool minBucketBinarySearch = MinBucketBinarySearch; + static constexpr bool premerge = Premerge; + static constexpr bool collectMax = CollectMax; + static constexpr mathic::GeobucketBucketStorage bucketStorage = (mathic::GeobucketBucketStorage)BucketStorage; - static const size_t insertFactor = InsertFactor; + static constexpr size_t insertFactor = InsertFactor; }; template< diff --git a/src/pqsim/HeapModel.h b/src/pqsim/HeapModel.h index ad39299..a470026 100644 --- a/src/pqsim/HeapModel.h +++ b/src/pqsim/HeapModel.h @@ -6,7 +6,7 @@ template struct HeapModelBase { - static const bool fastIndex = FastIndex; + static constexpr bool fastIndex = FastIndex; }; template diff --git a/src/pqsim/Model.h b/src/pqsim/Model.h index 12767d5..2975b86 100644 --- a/src/pqsim/Model.h +++ b/src/pqsim/Model.h @@ -96,7 +96,7 @@ class Model { std::vector& getPending() {return _pending;} size_t getComparisons() const {return _comparisons;} - static const bool supportDeduplication = Deduplicate; + static constexpr bool supportDeduplication = Deduplicate; private: mutable size_t _comparisons; diff --git a/src/pqsim/TourTreeModel.h b/src/pqsim/TourTreeModel.h index 109a913..34ec53c 100644 --- a/src/pqsim/TourTreeModel.h +++ b/src/pqsim/TourTreeModel.h @@ -6,7 +6,7 @@ template struct TourTreeModelBase { - static const bool fastIndex = FastIndex; + static constexpr bool fastIndex = FastIndex; }; template diff --git a/src/test/PairQueue.cpp b/src/test/PairQueue.cpp index c962a4a..38dfa12 100644 --- a/src/test/PairQueue.cpp +++ b/src/test/PairQueue.cpp @@ -40,7 +40,7 @@ namespace mathic { namespace PairQueueNamespace { template<> struct SupportRetirement { - static bool const value = false; + static constexpr bool value = false; }; } } @@ -329,7 +329,7 @@ namespace mathic { template<> struct SupportRetirement { - static bool const value = true; + static constexpr bool value = true; }; } } From e9f74601c942a8f252cd934d8024e0e1f00f0de9 Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Wed, 26 Aug 2026 12:08:39 -0400 Subject: [PATCH 3/4] Raise an error if we compile with < C++17 Mathic's headers declare static constexpr data members and rely on C++17 making those implicitly inline; without it, odr-using one fails to link for want of an out-of-line definition. Diagnose a too-old -std here rather than leaving the user with a confusing undefined reference. --- src/mathic/stdinc.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mathic/stdinc.h b/src/mathic/stdinc.h index c72950b..3e4f16c 100644 --- a/src/mathic/stdinc.h +++ b/src/mathic/stdinc.h @@ -1,6 +1,10 @@ #ifndef MATHIC_STDINC_GUARD #define MATHIC_STDINC_GUARD +#if __cplusplus < 201703L +#error "mathic requires C++17 or later" +#endif + #ifdef _MSC_VER // For Microsoft Compiler in Visual Studio C++. // Sometimes you know that a function will be called very rarely so you want to // tell the compiler not to inline it even if it could be inlined at only a From e2c5cbace4e2e5ad153e15be9fc00fc801b078de Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Wed, 26 Aug 2026 12:24:46 -0400 Subject: [PATCH 4/4] Re-enable a test now that we've fixed the underlying bug --- src/test/PairQueue.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/PairQueue.cpp b/src/test/PairQueue.cpp index 38dfa12..1696cfc 100644 --- a/src/test/PairQueue.cpp +++ b/src/test/PairQueue.cpp @@ -396,8 +396,7 @@ TEST(PairQueue, ConDeconOfPairData) { } TEST(PairQueue, RetirementSetToTrue) { - // This test gives an undesigned symbol in the linker now - // ASSERT_TRUE(mathic::PairQueue::SupportRetirement); + ASSERT_TRUE(mathic::PairQueue::SupportRetirement); mathic::PairQueue pq((PQConDeconCounterConf())); pq.configuration().owningQueue = &pq; mathic::PairQueue::Index* null = 0;