Merge pull request #296 from WireCell/fix-spdlog-fmtlib

Fix spdlog fmtlib

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/Spdlog.h"
 #include <ostream>
 #include <functional>
 
@@ -71,4 +72,6 @@ namespace std {
     };
 }  // namespace std
 
+template <> struct fmt::formatter<WireCell::WirePlaneId> : fmt::ostream_formatter {};
+
 #endif

img/inc/WireCellImg/PointCloudFacade.h

@@ -9,6 +9,7 @@
 #include "WireCellUtil/PointTree.h"
 #include "WireCellUtil/Point.h"
 #include "WireCellUtil/Units.h"
+#include "WireCellUtil/Spdlog.h"
 
 // using namespace WireCell;  NO!  do not open up namespaces in header files!
 
@@ -367,4 +368,8 @@ namespace WireCell::PointCloud::Facade {
 
 }  // namespace WireCell::PointCloud::Facade
 
+template <> struct fmt::formatter<WireCell::PointCloud::Facade::Blob> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<WireCell::PointCloud::Facade::Cluster> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<WireCell::PointCloud::Facade::Grouping> : fmt::ostream_formatter {};
+
 #endif

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;

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<node_element_t>();
@@ -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;
 }
 

util/inc/WireCellUtil/Array.h

@@ -27,6 +27,7 @@
 
 #include "WireCellUtil/Waveform.h"
 #include "WireCellUtil/Eigen.h"
+#include "WireCellUtil/Spdlog.h"
 
 #include <memory>
 #include <vector>
@@ -99,4 +100,7 @@ namespace WireCell {
     }  // namespace Array
 }  // namespace WireCell
 
+template <> struct fmt::formatter<Eigen::Matrix<double,-1,-1>> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<Eigen::Matrix<double,-1, 1>> : fmt::ostream_formatter {};
+
 #endif

util/inc/WireCellUtil/Configuration.h

@@ -1,6 +1,8 @@
 #ifndef WIRECELL_CONFIGURATION
 #define WIRECELL_CONFIGURATION
 
+#include "WireCellUtil/Spdlog.h"
+
 #include <json/json.h>
 #include <boost/algorithm/string/split.hpp>
 #include <boost/algorithm/string/classification.hpp>
@@ -211,4 +213,6 @@ namespace WireCell {
 
 }  // namespace WireCell
 
+template <> struct fmt::formatter<WireCell::Configuration> : fmt::ostream_formatter {};
+
 #endif

util/inc/WireCellUtil/Logging.h

@@ -16,8 +16,7 @@
 // #define SPDLOG_ACTIVE_LEVEL SPDLOG_LEVEL_DEBUG
 #include "WireCellUtil/BuildConfig.h"
 
-#include "spdlog/spdlog.h"
-#include "spdlog/fmt/ostr.h"
+#include "WireCellUtil/Spdlog.h"
 
 #include <memory>
 #include <string>

util/inc/WireCellUtil/Measurement.h

@@ -14,6 +14,8 @@
 #ifndef WIRECELLUTIL_MEASUREMENT
 #define WIRECELLUTIL_MEASUREMENT
 
+#include "WireCellUtil/Spdlog.h"
+
 #include "WireCellUtil/boost_units_measurement.h"
 
 namespace WireCell::Measurement {
@@ -23,4 +25,7 @@ namespace WireCell::Measurement {
 
 }
 
+template <> struct fmt::formatter<WireCell::Measurement::float32> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<WireCell::Measurement::float64> : fmt::ostream_formatter {};
+
 #endif

util/inc/WireCellUtil/NaryTesting.h

@@ -4,6 +4,7 @@
 #define WIRECELLUTIL_NARYTESTING
 
 #include "WireCellUtil/NaryTreeNotified.h"
+#include "WireCellUtil/Spdlog.h"
 
 namespace WireCell::NaryTesting {
 
@@ -53,5 +54,6 @@ namespace WireCell::NaryTesting {
 
 }
 
+template <> struct fmt::formatter<WireCell::NaryTesting::Introspective> : fmt::ostream_formatter {};
 
 #endif

util/inc/WireCellUtil/Point.h

@@ -3,6 +3,7 @@
 
 #include "WireCellUtil/D3Vector.h"
 #include "WireCellUtil/Configuration.h"
+#include "WireCellUtil/Spdlog.h"
 
 #include <set>
 #include <memory>  // auto_ptr
@@ -126,4 +127,7 @@ namespace WireCell {
 // }
 
 
+template <> struct fmt::formatter<WireCell::Point> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<WireCell::Ray> : fmt::ostream_formatter {};
+
 #endif

util/inc/WireCellUtil/PointTree.h

@@ -319,4 +319,6 @@ namespace WireCell::PointCloud::Tree {
 
 }
 
+template <> struct fmt::formatter<WireCell::PointCloud::Tree::Scope> : fmt::ostream_formatter {};
+
 #endif

util/inc/WireCellUtil/RayGrid.h

@@ -12,6 +12,8 @@
 #include "WireCellUtil/Point.h"
 #include "WireCellUtil/ObjectArray2d.h"
 #include "WireCellUtil/MultiArray.h"
+#include "WireCellUtil/Spdlog.h"
+
 #include <vector>
 #include <map>
 
@@ -168,4 +170,6 @@ namespace WireCell {
     }  // namespace RayGrid
 }  // namespace WireCell
 
+template <> struct fmt::formatter<WireCell::RayGrid::coordinate_t> : fmt::ostream_formatter {};
+
 #endif

util/inc/WireCellUtil/RayTiling.h

@@ -231,4 +231,8 @@ namespace WireCell {
     }  // namespace RayGrid
 }  // namespace WireCell
 
+template <> struct fmt::formatter<WireCell::RayGrid::Activity> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<WireCell::RayGrid::Blob> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<WireCell::RayGrid::Strip> : fmt::ostream_formatter {};
+
 #endif

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 <spdlog/spdlog.h>
+
+// 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 <fmt/core.h>
+#else
+#include <spdlog/fmt/bundled/core.h>
+#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<TYPE> : fmt::ostream_formatter {};
+//
+// while backporting that base type to old fmtlib.
+#if FMT_VERSION >= 90000
+#include <fmt/ostream.h>
+// This provides fmt::ostream_formatter.
+#else
+#include <spdlog/fmt/ostr.h>
+// This id copied from fmt/ostream.h from fmtlib 10.2 with const_cast tweak.
+namespace fmt {
+    template <typename Char>
+    struct basic_ostream_formatter : formatter<basic_string_view<Char>, Char> {
+        void set_debug_format() = delete;
+
+        template <typename T, typename OutputIt>
+        auto format(const T& value, basic_format_context<OutputIt, Char>& ctx) const
+            -> OutputIt {
+            auto buffer = basic_memory_buffer<Char>();
+            detail::format_value(buffer, value);
+            // Must const_cast as apparently the format() method from fmtlib 7.1.3 is not const.
+            return const_cast<basic_ostream_formatter<Char>*>(this)->formatter<basic_string_view<Char>, Char>::format(
+                {buffer.data(), buffer.size()}, ctx);
+        }
+    };
+    using ostream_formatter = basic_ostream_formatter<char>;
+}
+#endif
+
+
+#endif

util/test/doctest_eigen_pca.cxx

@@ -2,10 +2,6 @@
 #include "WireCellUtil/Array.h"
 #include "WireCellUtil/doctest.h"
 
-
-
-#include <iostream>
-
 using namespace Eigen;
 
 void dump(std::string msg,

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 [email protected]+shared ^[email protected]
+   spack install [email protected]+shared ^[email protected]
+   spack install [email protected]+shared ^[email protected]
+   spack install [email protected]+shared ^[email protected]
+   spack install [email protected]+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-<version>/local spdlog@<version>
+   echo 'load_prefix local' > spdlog-<version>/.envrc
+   cd spdlog-<version>/
+   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 <iostream>
+namespace NS {
+
+    struct S { int x; };
+
+    inline std::ostream& operator<<(std::ostream& os, const S& s)
+    {
+        os << "<S.x=" << s.x << ">";
+        return os;
+    }
+}
+
+#include <spdlog/spdlog.h>
+
+#if defined(SPDLOG_FMT_EXTERNAL)
+#include <fmt/core.h>
+#else
+#include <spdlog/fmt/bundled/core.h>
+#endif
+
+#if FMT_VERSION >= 90000
+#include <fmt/ostream.h>
+// This provides fmt::ostream_formatter.
+#else
+#include <spdlog/fmt/ostr.h>
+// This id copied from fmt/ostream.h from fmtlib 10.2 with const_cast tweak.
+namespace fmt {
+    template <typename Char>
+    struct basic_ostream_formatter : formatter<basic_string_view<Char>, Char> {
+        void set_debug_format() = delete;
+
+        template <typename T, typename OutputIt>
+        auto format(const T& value, basic_format_context<OutputIt, Char>& ctx) const
+            -> OutputIt {
+            auto buffer = basic_memory_buffer<Char>();
+            detail::format_value(buffer, value);
+            // Must const_cast as apparently the format() method from fmtlib 7.1.3 is not const.
+            return const_cast<basic_ostream_formatter<Char>*>(this)->formatter<basic_string_view<Char>, Char>::format(
+                {buffer.data(), buffer.size()}, ctx);
+        }
+    };
+    using ostream_formatter = basic_ostream_formatter<char>;
+}
+#endif
+
+
+// The goal with the mess above is to make this "new style" requirement work with old fmtlib.
+template <> struct fmt::formatter<NS::S> : fmt::ostream_formatter {};
+
+
+
+int main()
+{
+    NS::S s{42};
+    std::cerr << s << "\n";
+    spdlog::info("{}", s);
+    spdlog::info("FMT_VERSION={}", FMT_VERSION);
+
+    return 0;
+}

util/test/test_rayclustering.cxx

@@ -29,6 +29,7 @@ const double height = 100;
 
 #include "raygrid_dump.h"
 
+
 static std::vector<Point> make_points(std::default_random_engine& generator, double x)
 {
     std::vector<Point> points;
@@ -151,7 +152,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 +162,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 +174,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);

wscript

@@ -40,6 +40,9 @@ def options(opt):
                    choices = log_levels,
                    help="The compiled minimum log level for SPDLOG_<LEVEL>() macros (def=info)")
 
+    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)
@@ -72,6 +75,9 @@ int main(int argc,const char *argv[])
                   mandatory=False)
 
 
+    # cfg.env.CXXFLAGS += ['-Wpedantic', '-Werror']
+    cfg.env.CXXFLAGS += ['-std='+cfg.options.cxxstd.lower()]
+
     cfg.env.CXXFLAGS += ['-std=c++17']