From 552ebe54e920c766e12a0f2152efdd774ec48b39 Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Tue, 28 May 2024 12:26:38 -0400 Subject: [PATCH 1/7] Give a CLI option to set C++ standard version --- wscript | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wscript b/wscript index 02b11f76d..67a7d3ba1 100644 --- a/wscript +++ b/wscript @@ -33,6 +33,9 @@ def options(opt): opt.add_option('--with-spdlog-static', type=str, default="yes", help="Def is true, set to false if your spdlog is not compiled (not recomended)") + opt.add_option('--cxxstd', default='c++17', + help="Set the value for the compiler's --std= option, default 'c++17'") + def configure(cfg): # Save to BuildConfig.h and env cfg.define("WIRECELL_VERSION", VERSION) @@ -64,7 +67,7 @@ int main(int argc,const char *argv[]) cfg.env.CXXFLAGS += ['-Wno-deprecated-declarations'] cfg.env.CXXFLAGS += ['-Wall', '-Wno-unused-local-typedefs', '-Wno-unused-function'] # cfg.env.CXXFLAGS += ['-Wpedantic', '-Werror'] - cfg.env.CXXFLAGS += ['-std=c++17'] + cfg.env.CXXFLAGS += ['-std='+cfg.options.cxxstd.lower()] if cfg.options.with_spdlog_static.lower() in ("yes","on","true"): cfg.env.CXXFLAGS += ['-DSPDLOG_COMPILED_LIB=1'] From b3e37e8802f7c98c6886612d3f3f16e63e14566b Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Tue, 28 May 2024 12:27:59 -0400 Subject: [PATCH 2/7] Hopefully fix https://github.com/WireCell/wire-cell-spack/issues/12 --- iface/inc/WireCellIface/WirePlaneId.h | 6 ++++++ pgraph/src/Factory.cxx | 2 +- util/inc/WireCellUtil/Array.h | 8 ++++++++ util/inc/WireCellUtil/Configuration.h | 6 ++++++ util/inc/WireCellUtil/Logging.h | 4 ++-- util/inc/WireCellUtil/Measurement.h | 7 +++++++ util/inc/WireCellUtil/NaryTesting.h | 5 +++++ util/inc/WireCellUtil/Point.h | 8 ++++++++ util/inc/WireCellUtil/PointTree.h | 6 ++++++ util/inc/WireCellUtil/RayTiling.h | 8 ++++++++ util/test/doctest_eigen_pca.cxx | 4 ---- 11 files changed, 57 insertions(+), 7 deletions(-) diff --git a/iface/inc/WireCellIface/WirePlaneId.h b/iface/inc/WireCellIface/WirePlaneId.h index 8578cb515..4494e17b7 100644 --- a/iface/inc/WireCellIface/WirePlaneId.h +++ b/iface/inc/WireCellIface/WirePlaneId.h @@ -71,4 +71,10 @@ namespace std { }; } // namespace std +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/pgraph/src/Factory.cxx b/pgraph/src/Factory.cxx index 49cedc567..1681feaed 100644 --- a/pgraph/src/Factory.cxx +++ b/pgraph/src/Factory.cxx @@ -33,7 +33,7 @@ Node* Factory::operator()(WireCell::INode::pointer wcnode) } auto mit = m_factory.find(wcnode->category()); if (mit == m_factory.end()) { - l->critical("factory failed to find maker for category: {}", wcnode->category()); + l->critical("factory failed to find maker for category: {}", (int)wcnode->category()); THROW(ValueError() << errmsg{"failed to find maker"}); } auto maker = mit->second; diff --git a/util/inc/WireCellUtil/Array.h b/util/inc/WireCellUtil/Array.h index 2618c0d0b..0659fa89a 100644 --- a/util/inc/WireCellUtil/Array.h +++ b/util/inc/WireCellUtil/Array.h @@ -101,4 +101,12 @@ namespace WireCell { } // namespace Array } // namespace WireCell +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter> : fmt::ostream_formatter {}; +template <> struct fmt::formatter> : fmt::ostream_formatter {}; +#endif + + #endif diff --git a/util/inc/WireCellUtil/Configuration.h b/util/inc/WireCellUtil/Configuration.h index 574e453fb..3cd4d64b9 100644 --- a/util/inc/WireCellUtil/Configuration.h +++ b/util/inc/WireCellUtil/Configuration.h @@ -211,4 +211,10 @@ namespace WireCell { } // namespace WireCell +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/util/inc/WireCellUtil/Logging.h b/util/inc/WireCellUtil/Logging.h index 4103fb12a..e7eaf1628 100644 --- a/util/inc/WireCellUtil/Logging.h +++ b/util/inc/WireCellUtil/Logging.h @@ -11,8 +11,8 @@ // CLI are higher. #define SPDLOG_ACTIVE_LEVEL SPDLOG_LEVEL_TRACE -#include "spdlog/spdlog.h" -#include "spdlog/fmt/ostr.h" +#include +#include #include #include diff --git a/util/inc/WireCellUtil/Measurement.h b/util/inc/WireCellUtil/Measurement.h index be24f99c5..6f033a987 100644 --- a/util/inc/WireCellUtil/Measurement.h +++ b/util/inc/WireCellUtil/Measurement.h @@ -23,4 +23,11 @@ namespace WireCell::Measurement { } +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/util/inc/WireCellUtil/NaryTesting.h b/util/inc/WireCellUtil/NaryTesting.h index b00863b21..b59c27f4f 100644 --- a/util/inc/WireCellUtil/NaryTesting.h +++ b/util/inc/WireCellUtil/NaryTesting.h @@ -53,5 +53,10 @@ namespace WireCell::NaryTesting { } +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif #endif diff --git a/util/inc/WireCellUtil/Point.h b/util/inc/WireCellUtil/Point.h index 784f5afc5..b58adf706 100644 --- a/util/inc/WireCellUtil/Point.h +++ b/util/inc/WireCellUtil/Point.h @@ -125,4 +125,12 @@ namespace WireCell { // return WireCell::Ray(ray.first*scale, ray.second*scale); // } +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + + #endif diff --git a/util/inc/WireCellUtil/PointTree.h b/util/inc/WireCellUtil/PointTree.h index 54d4af7e3..49a70ef78 100644 --- a/util/inc/WireCellUtil/PointTree.h +++ b/util/inc/WireCellUtil/PointTree.h @@ -282,4 +282,10 @@ namespace WireCell::PointCloud::Tree { } +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/util/inc/WireCellUtil/RayTiling.h b/util/inc/WireCellUtil/RayTiling.h index 8fb161e6d..dc2300207 100644 --- a/util/inc/WireCellUtil/RayTiling.h +++ b/util/inc/WireCellUtil/RayTiling.h @@ -231,4 +231,12 @@ namespace WireCell { } // namespace RayGrid } // namespace WireCell +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/util/test/doctest_eigen_pca.cxx b/util/test/doctest_eigen_pca.cxx index ed0667064..62c8c3d34 100644 --- a/util/test/doctest_eigen_pca.cxx +++ b/util/test/doctest_eigen_pca.cxx @@ -2,10 +2,6 @@ #include "WireCellUtil/Array.h" #include "WireCellUtil/doctest.h" - - -#include - using namespace Eigen; void dump(std::string msg, From abf845af201e2455df661fc845c79b3209cfad04 Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Tue, 28 May 2024 13:31:01 -0400 Subject: [PATCH 3/7] More spdlog/fmtlib fixes after merging in new code from master --- img/inc/WireCellImg/PointCloudFacade.h | 8 ++++++++ sio/src/ClusterFileSource.cxx | 4 ++-- util/inc/WireCellUtil/RayGrid.h | 6 ++++++ util/test/test_rayclustering.cxx | 8 +++++--- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/img/inc/WireCellImg/PointCloudFacade.h b/img/inc/WireCellImg/PointCloudFacade.h index a0dc231a1..aa628b37a 100644 --- a/img/inc/WireCellImg/PointCloudFacade.h +++ b/img/inc/WireCellImg/PointCloudFacade.h @@ -367,4 +367,12 @@ namespace WireCell::PointCloud::Facade { } // namespace WireCell::PointCloud::Facade +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter : fmt::ostream_formatter {}; +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/sio/src/ClusterFileSource.cxx b/sio/src/ClusterFileSource.cxx index 718fd72f1..4f62b59c9 100644 --- a/sio/src/ClusterFileSource.cxx +++ b/sio/src/ClusterFileSource.cxx @@ -184,7 +184,7 @@ ICluster::pointer ClusterFileSource::load_numpy(int ident) THROW(ValueError() << errmsg{"illegal shape"}); } log->debug("file {} with type={} code={} ident={} shape=({},{})", - m_cur.fname, pf.type, pf.code, ident, shape[0], shape[1]); + m_cur.fname, (int)pf.type, pf.code, ident, shape[0], shape[1]); if (pf.type == ParsedFilename::node) { const node_element_t* data = pig.as_type(); @@ -292,7 +292,7 @@ ICluster::pointer ClusterFileSource::dispatch() return load_numpy(pf.ident); } log->warn("do not know how to dispatch file {} with type={} code={} ident={}", - m_cur.fname, pf.type, pf.code, pf.ident); + m_cur.fname, (int)pf.type, pf.code, pf.ident); return nullptr; } diff --git a/util/inc/WireCellUtil/RayGrid.h b/util/inc/WireCellUtil/RayGrid.h index 2c77a521e..022a32131 100644 --- a/util/inc/WireCellUtil/RayGrid.h +++ b/util/inc/WireCellUtil/RayGrid.h @@ -168,4 +168,10 @@ namespace WireCell { } // namespace RayGrid } // namespace WireCell +#include +#if FMT_VERSION >= 90000 +#include +template <> struct fmt::formatter : fmt::ostream_formatter {}; +#endif + #endif diff --git a/util/test/test_rayclustering.cxx b/util/test/test_rayclustering.cxx index 3843191b6..7d69a7209 100644 --- a/util/test/test_rayclustering.cxx +++ b/util/test/test_rayclustering.cxx @@ -1,5 +1,6 @@ #include "WireCellUtil/RayClustering.h" #include "WireCellUtil/RayTiling.h" +#include #include "WireCellUtil/Waveform.h" #include "WireCellUtil/Testing.h" #include "WireCellUtil/Logging.h" @@ -29,6 +30,7 @@ const double height = 100; #include "raygrid_dump.h" + static std::vector make_points(std::default_random_engine& generator, double x) { std::vector points; @@ -151,7 +153,7 @@ struct Chirp { for (layer_index_t layer = 0; layer < nlayers; ++layer) { const auto& astrip = astrips[layer]; if (layer == c.first.layer) { - info("L{} A: {} {}", layer, astrip, c); + info("L{} A: {} {},{}", layer, astrip, c.first, c.second); if (astrip.on(c.first.grid)) { info("\ton with found={} nlayers={}", found, nlayers); ++found; @@ -161,7 +163,7 @@ struct Chirp { break; } if (layer == c.second.layer) { - info("L{} A: {} {}", layer, astrip, c); + info("L{} A: {} {},{}", layer, astrip, c.first, c.second); if (astrip.on(c.second.grid)) { info("\ton with found={} nlayers={}", found, nlayers); ++found; @@ -173,7 +175,7 @@ struct Chirp { const double ploc = coords.pitch_location(c.first, c.second, layer); const int pind = coords.pitch_index(ploc, layer); - info("L{} A: {} pind={} ploc={} {}", layer, astrip, pind, ploc, c); + info("L{} A: {} pind={} ploc={} {},{}", layer, astrip, pind, ploc, c.first, c.second); if (astrip.in(pind)) { info("\tin with found={} nlayers={}", found, nlayers); From 88288fc7be8c9f39ca9e0b1d0c87ac0d9b4ceee6 Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Tue, 28 May 2024 15:02:36 -0400 Subject: [PATCH 4/7] Remove old CXXFLAGS setting that snuck in through the recent merge. --- wscript | 3 --- 1 file changed, 3 deletions(-) diff --git a/wscript b/wscript index 59ab188f2..a558ad2e2 100644 --- a/wscript +++ b/wscript @@ -75,9 +75,6 @@ int main(int argc,const char *argv[]) mandatory=False) - # boost 1.59 uses auto_ptr and GCC 5 deprecates it vociferously. - cfg.env.CXXFLAGS += ['-Wno-deprecated-declarations'] - cfg.env.CXXFLAGS += ['-Wall', '-Wno-unused-local-typedefs', '-Wno-unused-function'] # cfg.env.CXXFLAGS += ['-Wpedantic', '-Werror'] cfg.env.CXXFLAGS += ['-std='+cfg.options.cxxstd.lower()] From fc8fb820fb60c0683bf6dddd7b451d3796cef169 Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Tue, 28 May 2024 16:14:43 -0400 Subject: [PATCH 5/7] Try harder to do nothing with old spdlog and built-in fmtlib --- iface/inc/WireCellIface/WirePlaneId.h | 5 +--- img/inc/WireCellImg/PointCloudFacade.h | 5 +--- util/inc/WireCellUtil/Array.h | 6 +---- util/inc/WireCellUtil/Configuration.h | 6 ++--- util/inc/WireCellUtil/FmtLib.h | 32 ++++++++++++++++++++++++++ util/inc/WireCellUtil/Measurement.h | 6 ++--- util/inc/WireCellUtil/NaryTesting.h | 5 +--- util/inc/WireCellUtil/Point.h | 7 ++---- util/inc/WireCellUtil/PointTree.h | 6 ++--- util/inc/WireCellUtil/RayGrid.h | 5 +--- util/inc/WireCellUtil/RayTiling.h | 5 +--- 11 files changed, 46 insertions(+), 42 deletions(-) create mode 100644 util/inc/WireCellUtil/FmtLib.h diff --git a/iface/inc/WireCellIface/WirePlaneId.h b/iface/inc/WireCellIface/WirePlaneId.h index 4494e17b7..c24dd2083 100644 --- a/iface/inc/WireCellIface/WirePlaneId.h +++ b/iface/inc/WireCellIface/WirePlaneId.h @@ -3,6 +3,7 @@ // fixme: should move into WirePlaneIdCfg.h or similar. (more below) #include "WireCellUtil/Configuration.h" +#include "WireCellUtil/FmtLib.h" #include #include @@ -71,10 +72,6 @@ namespace std { }; } // namespace std -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/img/inc/WireCellImg/PointCloudFacade.h b/img/inc/WireCellImg/PointCloudFacade.h index aa628b37a..a319b9608 100644 --- a/img/inc/WireCellImg/PointCloudFacade.h +++ b/img/inc/WireCellImg/PointCloudFacade.h @@ -9,6 +9,7 @@ #include "WireCellUtil/PointTree.h" #include "WireCellUtil/Point.h" #include "WireCellUtil/Units.h" +#include "WireCellUtil/FmtLib.h" // using namespace WireCell; NO! do not open up namespaces in header files! @@ -367,12 +368,8 @@ namespace WireCell::PointCloud::Facade { } // namespace WireCell::PointCloud::Facade -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; template <> struct fmt::formatter : fmt::ostream_formatter {}; template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/util/inc/WireCellUtil/Array.h b/util/inc/WireCellUtil/Array.h index 7b41acd4f..bd482f346 100644 --- a/util/inc/WireCellUtil/Array.h +++ b/util/inc/WireCellUtil/Array.h @@ -27,6 +27,7 @@ #include "WireCellUtil/Waveform.h" #include "WireCellUtil/Eigen.h" +#include "WireCellUtil/FmtLib.h" #include #include @@ -99,12 +100,7 @@ namespace WireCell { } // namespace Array } // namespace WireCell -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter> : fmt::ostream_formatter {}; template <> struct fmt::formatter> : fmt::ostream_formatter {}; -#endif - #endif diff --git a/util/inc/WireCellUtil/Configuration.h b/util/inc/WireCellUtil/Configuration.h index 3cd4d64b9..d4cf5c8ee 100644 --- a/util/inc/WireCellUtil/Configuration.h +++ b/util/inc/WireCellUtil/Configuration.h @@ -1,6 +1,8 @@ #ifndef WIRECELL_CONFIGURATION #define WIRECELL_CONFIGURATION +#include "WireCellUtil/FmtLib.h" + #include #include #include @@ -211,10 +213,6 @@ namespace WireCell { } // namespace WireCell -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/util/inc/WireCellUtil/FmtLib.h b/util/inc/WireCellUtil/FmtLib.h new file mode 100644 index 000000000..bc5dd5359 --- /dev/null +++ b/util/inc/WireCellUtil/FmtLib.h @@ -0,0 +1,32 @@ +/** + Recent SPDLOG can be compiled with newer, external fmtlib and that no longer + uses ostream insertion operator without us making a fmt::formatter<>. OTOH, + this form doesn't exist in older fmtlib such as included in SPDLOG. + + So, here define a dummy formatter base when old or spdlog fmtlib is used. + + See https://github.com/WireCell/wire-cell-spack/issues/12 + + */ + +#ifndef WIRECELL_FMTLIB_H +#define WIRECELL_FMTLIB_H + +#ifdef SPDLOG_FMT_EXTERNAL +#include +#if FMT_VERSION >= 90000 +#include +#else +namespace fmt { + struct ostream_formatter {}; + template struct formatter {}; +} +#endif +#else +namespace fmt { + struct ostream_formatter {}; + template struct formatter {}; +} +#endif + +#endif diff --git a/util/inc/WireCellUtil/Measurement.h b/util/inc/WireCellUtil/Measurement.h index 6f033a987..dcd4e397f 100644 --- a/util/inc/WireCellUtil/Measurement.h +++ b/util/inc/WireCellUtil/Measurement.h @@ -14,6 +14,8 @@ #ifndef WIRECELLUTIL_MEASUREMENT #define WIRECELLUTIL_MEASUREMENT +#include "WireCellUtil/FmtLib.h" + #include "WireCellUtil/boost_units_measurement.h" namespace WireCell::Measurement { @@ -23,11 +25,7 @@ namespace WireCell::Measurement { } -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/util/inc/WireCellUtil/NaryTesting.h b/util/inc/WireCellUtil/NaryTesting.h index 3ce446e1a..7c4979a08 100644 --- a/util/inc/WireCellUtil/NaryTesting.h +++ b/util/inc/WireCellUtil/NaryTesting.h @@ -4,6 +4,7 @@ #define WIRECELLUTIL_NARYTESTING #include "WireCellUtil/NaryTreeNotified.h" +#include "WireCellUtil/FmtLib.h" namespace WireCell::NaryTesting { @@ -53,10 +54,6 @@ namespace WireCell::NaryTesting { } -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/util/inc/WireCellUtil/Point.h b/util/inc/WireCellUtil/Point.h index b58adf706..690c148a8 100644 --- a/util/inc/WireCellUtil/Point.h +++ b/util/inc/WireCellUtil/Point.h @@ -3,6 +3,7 @@ #include "WireCellUtil/D3Vector.h" #include "WireCellUtil/Configuration.h" +#include "WireCellUtil/FmtLib.h" #include #include // auto_ptr @@ -125,12 +126,8 @@ namespace WireCell { // return WireCell::Ray(ray.first*scale, ray.second*scale); // } -#include -#if FMT_VERSION >= 90000 -#include + template <> struct fmt::formatter : fmt::ostream_formatter {}; template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif - #endif diff --git a/util/inc/WireCellUtil/PointTree.h b/util/inc/WireCellUtil/PointTree.h index 3c5a96ac3..2dd7b0a4d 100644 --- a/util/inc/WireCellUtil/PointTree.h +++ b/util/inc/WireCellUtil/PointTree.h @@ -13,6 +13,8 @@ #include "WireCellUtil/NaryTreeFacade.h" #include "WireCellUtil/KDTree.h" +#include "WireCellUtil/FmtLib.h" + #include "WireCellUtil/Logging.h" // debug #include @@ -319,10 +321,6 @@ namespace WireCell::PointCloud::Tree { } -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/util/inc/WireCellUtil/RayGrid.h b/util/inc/WireCellUtil/RayGrid.h index 022a32131..b911f7c23 100644 --- a/util/inc/WireCellUtil/RayGrid.h +++ b/util/inc/WireCellUtil/RayGrid.h @@ -12,6 +12,7 @@ #include "WireCellUtil/Point.h" #include "WireCellUtil/ObjectArray2d.h" #include "WireCellUtil/MultiArray.h" +#include "WireCellUtil/FmtLib.h" #include #include @@ -168,10 +169,6 @@ namespace WireCell { } // namespace RayGrid } // namespace WireCell -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif diff --git a/util/inc/WireCellUtil/RayTiling.h b/util/inc/WireCellUtil/RayTiling.h index dc2300207..fc2a0d4e5 100644 --- a/util/inc/WireCellUtil/RayTiling.h +++ b/util/inc/WireCellUtil/RayTiling.h @@ -13,6 +13,7 @@ #include #include "WireCellUtil/RayGrid.h" +#include "WireCellUtil/FmtLib.h" namespace WireCell { @@ -231,12 +232,8 @@ namespace WireCell { } // namespace RayGrid } // namespace WireCell -#include -#if FMT_VERSION >= 90000 -#include template <> struct fmt::formatter : fmt::ostream_formatter {}; template <> struct fmt::formatter : fmt::ostream_formatter {}; template <> struct fmt::formatter : fmt::ostream_formatter {}; -#endif #endif From b7aa341d0bba2235202b57954d2d6a98c2160289 Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Wed, 29 May 2024 13:58:44 -0400 Subject: [PATCH 6/7] Rework previous fix with guidance from actual testing on old spdlog/fmtlib versions --- iface/inc/WireCellIface/WirePlaneId.h | 2 +- img/inc/WireCellImg/PointCloudFacade.h | 2 +- util/inc/WireCellUtil/Array.h | 2 +- util/inc/WireCellUtil/Configuration.h | 2 +- util/inc/WireCellUtil/FmtLib.h | 32 --------- util/inc/WireCellUtil/Logging.h | 3 +- util/inc/WireCellUtil/Measurement.h | 2 +- util/inc/WireCellUtil/NaryTesting.h | 2 +- util/inc/WireCellUtil/Point.h | 2 +- util/inc/WireCellUtil/PointTree.h | 2 - util/inc/WireCellUtil/RayGrid.h | 3 +- util/inc/WireCellUtil/RayTiling.h | 1 - util/inc/WireCellUtil/Spdlog.h | 57 ++++++++++++++++ util/test/test-spdlog-fmtlib.cxx | 93 ++++++++++++++++++++++++++ 14 files changed, 160 insertions(+), 45 deletions(-) delete mode 100644 util/inc/WireCellUtil/FmtLib.h create mode 100644 util/inc/WireCellUtil/Spdlog.h create mode 100644 util/test/test-spdlog-fmtlib.cxx diff --git a/iface/inc/WireCellIface/WirePlaneId.h b/iface/inc/WireCellIface/WirePlaneId.h index c24dd2083..687f9e39f 100644 --- a/iface/inc/WireCellIface/WirePlaneId.h +++ b/iface/inc/WireCellIface/WirePlaneId.h @@ -3,7 +3,7 @@ // fixme: should move into WirePlaneIdCfg.h or similar. (more below) #include "WireCellUtil/Configuration.h" -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" #include #include diff --git a/img/inc/WireCellImg/PointCloudFacade.h b/img/inc/WireCellImg/PointCloudFacade.h index a319b9608..8cdda0a50 100644 --- a/img/inc/WireCellImg/PointCloudFacade.h +++ b/img/inc/WireCellImg/PointCloudFacade.h @@ -9,7 +9,7 @@ #include "WireCellUtil/PointTree.h" #include "WireCellUtil/Point.h" #include "WireCellUtil/Units.h" -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" // using namespace WireCell; NO! do not open up namespaces in header files! diff --git a/util/inc/WireCellUtil/Array.h b/util/inc/WireCellUtil/Array.h index bd482f346..91a208253 100644 --- a/util/inc/WireCellUtil/Array.h +++ b/util/inc/WireCellUtil/Array.h @@ -27,7 +27,7 @@ #include "WireCellUtil/Waveform.h" #include "WireCellUtil/Eigen.h" -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" #include #include diff --git a/util/inc/WireCellUtil/Configuration.h b/util/inc/WireCellUtil/Configuration.h index d4cf5c8ee..0bd6a5f51 100644 --- a/util/inc/WireCellUtil/Configuration.h +++ b/util/inc/WireCellUtil/Configuration.h @@ -1,7 +1,7 @@ #ifndef WIRECELL_CONFIGURATION #define WIRECELL_CONFIGURATION -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" #include #include diff --git a/util/inc/WireCellUtil/FmtLib.h b/util/inc/WireCellUtil/FmtLib.h deleted file mode 100644 index bc5dd5359..000000000 --- a/util/inc/WireCellUtil/FmtLib.h +++ /dev/null @@ -1,32 +0,0 @@ -/** - Recent SPDLOG can be compiled with newer, external fmtlib and that no longer - uses ostream insertion operator without us making a fmt::formatter<>. OTOH, - this form doesn't exist in older fmtlib such as included in SPDLOG. - - So, here define a dummy formatter base when old or spdlog fmtlib is used. - - See https://github.com/WireCell/wire-cell-spack/issues/12 - - */ - -#ifndef WIRECELL_FMTLIB_H -#define WIRECELL_FMTLIB_H - -#ifdef SPDLOG_FMT_EXTERNAL -#include -#if FMT_VERSION >= 90000 -#include -#else -namespace fmt { - struct ostream_formatter {}; - template struct formatter {}; -} -#endif -#else -namespace fmt { - struct ostream_formatter {}; - template struct formatter {}; -} -#endif - -#endif diff --git a/util/inc/WireCellUtil/Logging.h b/util/inc/WireCellUtil/Logging.h index 1ee5f409f..ad5df150e 100644 --- a/util/inc/WireCellUtil/Logging.h +++ b/util/inc/WireCellUtil/Logging.h @@ -16,8 +16,7 @@ // #define SPDLOG_ACTIVE_LEVEL SPDLOG_LEVEL_DEBUG #include "WireCellUtil/BuildConfig.h" -#include -#include +#include "WireCellUtil/Spdlog.h" #include #include diff --git a/util/inc/WireCellUtil/Measurement.h b/util/inc/WireCellUtil/Measurement.h index dcd4e397f..9ebeb5f47 100644 --- a/util/inc/WireCellUtil/Measurement.h +++ b/util/inc/WireCellUtil/Measurement.h @@ -14,7 +14,7 @@ #ifndef WIRECELLUTIL_MEASUREMENT #define WIRECELLUTIL_MEASUREMENT -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" #include "WireCellUtil/boost_units_measurement.h" diff --git a/util/inc/WireCellUtil/NaryTesting.h b/util/inc/WireCellUtil/NaryTesting.h index 7c4979a08..5dc4f360a 100644 --- a/util/inc/WireCellUtil/NaryTesting.h +++ b/util/inc/WireCellUtil/NaryTesting.h @@ -4,7 +4,7 @@ #define WIRECELLUTIL_NARYTESTING #include "WireCellUtil/NaryTreeNotified.h" -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" namespace WireCell::NaryTesting { diff --git a/util/inc/WireCellUtil/Point.h b/util/inc/WireCellUtil/Point.h index 690c148a8..2c8fcd409 100644 --- a/util/inc/WireCellUtil/Point.h +++ b/util/inc/WireCellUtil/Point.h @@ -3,7 +3,7 @@ #include "WireCellUtil/D3Vector.h" #include "WireCellUtil/Configuration.h" -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" #include #include // auto_ptr diff --git a/util/inc/WireCellUtil/PointTree.h b/util/inc/WireCellUtil/PointTree.h index 2dd7b0a4d..198659915 100644 --- a/util/inc/WireCellUtil/PointTree.h +++ b/util/inc/WireCellUtil/PointTree.h @@ -13,8 +13,6 @@ #include "WireCellUtil/NaryTreeFacade.h" #include "WireCellUtil/KDTree.h" -#include "WireCellUtil/FmtLib.h" - #include "WireCellUtil/Logging.h" // debug #include diff --git a/util/inc/WireCellUtil/RayGrid.h b/util/inc/WireCellUtil/RayGrid.h index b911f7c23..45e86fe3d 100644 --- a/util/inc/WireCellUtil/RayGrid.h +++ b/util/inc/WireCellUtil/RayGrid.h @@ -12,7 +12,8 @@ #include "WireCellUtil/Point.h" #include "WireCellUtil/ObjectArray2d.h" #include "WireCellUtil/MultiArray.h" -#include "WireCellUtil/FmtLib.h" +#include "WireCellUtil/Spdlog.h" + #include #include diff --git a/util/inc/WireCellUtil/RayTiling.h b/util/inc/WireCellUtil/RayTiling.h index fc2a0d4e5..148d1bd03 100644 --- a/util/inc/WireCellUtil/RayTiling.h +++ b/util/inc/WireCellUtil/RayTiling.h @@ -13,7 +13,6 @@ #include #include "WireCellUtil/RayGrid.h" -#include "WireCellUtil/FmtLib.h" namespace WireCell { diff --git a/util/inc/WireCellUtil/Spdlog.h b/util/inc/WireCellUtil/Spdlog.h new file mode 100644 index 000000000..8cf9f4764 --- /dev/null +++ b/util/inc/WireCellUtil/Spdlog.h @@ -0,0 +1,57 @@ +/** + Include this file instead of directly including spdlog or fmtlib (fmt) + headers. + + It takes care of changing fmtlib API in the face of the fact that spdlog may + be compiled against an external fmtlib or use its "bundled" copy. + + More info: https://github.com/WireCell/wire-cell-spack/issues/12 + + */ + +#ifndef WIRECELLUTIL_SPDLOG +#define WIRECELLUTIL_SPDLOG + +#include + +// We need FMT version but it is found in core.h in different locations +// depending on if fmtlib is external to or bundled with spdlog. +#if defined(SPDLOG_FMT_EXTERNAL) +#include +#else +#include +#endif + +// Newer fmtlib no longer implicitly uses the ostream insertion operator<<. So +// we must have some ugliness to allow us to provide new, required specialization: +// +// template <> struct fmt::formatter : fmt::ostream_formatter {}; +// +// while backporting that base type to old fmtlib. +#if FMT_VERSION >= 90000 +#include +// This provides fmt::ostream_formatter. +#else +#include +// This id copied from fmt/ostream.h from fmtlib 10.2 with const_cast tweak. +namespace fmt { + template + struct basic_ostream_formatter : formatter, Char> { + void set_debug_format() = delete; + + template + auto format(const T& value, basic_format_context& ctx) const + -> OutputIt { + auto buffer = basic_memory_buffer(); + detail::format_value(buffer, value); + // Must const_cast as apparently the format() method from fmtlib 7.1.3 is not const. + return const_cast*>(this)->formatter, Char>::format( + {buffer.data(), buffer.size()}, ctx); + } + }; + using ostream_formatter = basic_ostream_formatter; +} +#endif + + +#endif diff --git a/util/test/test-spdlog-fmtlib.cxx b/util/test/test-spdlog-fmtlib.cxx new file mode 100644 index 000000000..1cd67a247 --- /dev/null +++ b/util/test/test-spdlog-fmtlib.cxx @@ -0,0 +1,93 @@ +/** + Test for fmtlib's change in how it handles (or doesn't handle) implicitly + using ostream insertion operator<< starting at fmtlib 9. + + Test like: + + spack install spdlog@1.8.2+shared ^fmt@7.1.3 + spack install spdlog@1.9.2+shared ^fmt@8.1.1 + spack install spdlog@1.10.0+shared ^fmt@8.1.1 + spack install spdlog@1.11.0+shared ^fmt@9.1.0 + spack install spdlog@1.13.0+shared + + The goal is to force a version of fmtlib near to the version of the bundled + fmtlib for a given spdlog version. + + For each, + + spack view add -i spdlog-/local spdlog@ + echo 'load_prefix local' > spdlog-/.envrc + cd spdlog-/ + direnv allow + + g++ -Wall -std=c++17 \ + -o test-spdlog-fmtlib \ + ../test-spdlog-fmtlib.cxx \ + (pkg-config spdlog --cflags --libs | string split ' ') + + (fish syntax) + + ./test-spdlog-fmtlib + + */ + +#include +namespace NS { + + struct S { int x; }; + + inline std::ostream& operator<<(std::ostream& os, const S& s) + { + os << ""; + return os; + } +} + +#include + +#if defined(SPDLOG_FMT_EXTERNAL) +#include +#else +#include +#endif + +#if FMT_VERSION >= 90000 +#include +// This provides fmt::ostream_formatter. +#else +#include +// This id copied from fmt/ostream.h from fmtlib 10.2 with const_cast tweak. +namespace fmt { + template + struct basic_ostream_formatter : formatter, Char> { + void set_debug_format() = delete; + + template + auto format(const T& value, basic_format_context& ctx) const + -> OutputIt { + auto buffer = basic_memory_buffer(); + detail::format_value(buffer, value); + // Must const_cast as apparently the format() method from fmtlib 7.1.3 is not const. + return const_cast*>(this)->formatter, Char>::format( + {buffer.data(), buffer.size()}, ctx); + } + }; + using ostream_formatter = basic_ostream_formatter; +} +#endif + + +// The goal with the mess above is to make this "new style" requirement work with old fmtlib. +template <> struct fmt::formatter : fmt::ostream_formatter {}; + + + +int main() +{ + NS::S s{42}; + std::cerr << s << "\n"; + spdlog::info("{}", s); + spdlog::info("FMT_VERSION={}", FMT_VERSION); + + return 0; +} From cbc5021b2760261bd20e3e469e864d76fadfd583 Mon Sep 17 00:00:00 2001 From: Brett Viren Date: Wed, 29 May 2024 16:16:25 -0400 Subject: [PATCH 7/7] Remove stray, unneeded inclusion of fmt/core.h --- util/test/test_rayclustering.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/util/test/test_rayclustering.cxx b/util/test/test_rayclustering.cxx index 7d69a7209..14d659821 100644 --- a/util/test/test_rayclustering.cxx +++ b/util/test/test_rayclustering.cxx @@ -1,6 +1,5 @@ #include "WireCellUtil/RayClustering.h" #include "WireCellUtil/RayTiling.h" -#include #include "WireCellUtil/Waveform.h" #include "WireCellUtil/Testing.h" #include "WireCellUtil/Logging.h"