fixed type conversion warnings & transparent pmtv::map_t comparator (#…

…109) * added more verbose compiler warnings ... goal is to get pmt spurious/unintentional warning free so that we can enable '-Werror' Signed-off-by: Ralph J. Steinhagen <[email protected]> * suppressed type-casting warnings Signed-off-by: Ralph J. Steinhagen <[email protected]> * fixed some type conversion warnings/errors Signed-off-by: Ralph J. Steinhagen <[email protected]> * added transparent comparator to pmtv::map_t == std::map<std::string, pmt, std::less<>> Signed-off-by: Ralph J. Steinhagen <[email protected]> --------- Signed-off-by: Ralph J. Steinhagen <[email protected]>
gnuradio · Jan 2, 2024 · 6b8a886 · 6b8a886
1 parent 1cda747
commit 6b8a886
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 22 deletions.
diff --git a/bench/bm_pmt_dict_pack_unpack.cpp b/bench/bm_pmt_dict_pack_unpack.cpp
@@ -16,11 +16,11 @@ bool run_test(const int32_t times, int32_t nitems)
     bool valid = true;
     for (int32_t i = 0; i < times; i++) {
         // Create the dictionary
-        std::map<std::string, pmt> starting_map;
+        pmtv::map_t starting_map;
 
 #if 1
         for (int32_t k = 0; k < nitems; k++) {
-            auto key = std::string("key" + std::to_string(k));
+            auto key = fmt::format("key{}", k);
             auto value = pmt(k);
 
             starting_map[key] = value;

diff --git a/bench/bm_pmt_dict_ref.cpp b/bench/bm_pmt_dict_ref.cpp
@@ -14,7 +14,7 @@ bool run_test(const int32_t times, pmt& d, int32_t index)
 {
     std::stringbuf sb; // fake channel
 
-    auto key = std::string("key" + std::to_string(index));
+    auto key = fmt::format("key{}", index);
 
     auto themap = pmtv::get_map(d);
 
@@ -50,7 +50,7 @@ int main(int argc, char* argv[])
 
     {
         // Create the dictionary
-        std::map<std::string, pmt> starting_map;
+        pmtv::map_t starting_map;
         for (int32_t k = 0; k < items; k++) {
             // auto key = std::string("key" + std::to_string(k));
             // auto value = scalar(k);

diff --git a/include/pmtv/pmt.hpp b/include/pmtv/pmt.hpp
@@ -8,8 +8,19 @@
 #include <cstddef>
 #include <ranges>
 #include <span>
+
+#ifdef __GNUC__
+#pragma GCC diagnostic push // ignore warning of external libraries that from this lib-context we do not have any control over
+#pragma GCC diagnostic ignored "-Wshadow"
+#pragma GCC diagnostic ignored "-Wsign-conversion"
+#endif
+
 #include <refl.hpp>
 
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 // Support for std::format is really spotty.
 // Gcc12 does not support it.
 // Eventually replace with std::format when that is widely available.
@@ -18,7 +29,7 @@
 namespace pmtv {
 
 using pmt = pmt_var_t;
-using map_t = std::map<std::string, pmt>;
+using map_t = std::map<std::string, pmt, std::less<>>;
 
 template <class T>
 inline constexpr std::in_place_type_t<std::vector<T>> vec_t{};
@@ -160,7 +171,7 @@ bool validate_map(const map_t& value, bool exact=false) {
         {
             using member_type = std::decay_t<decltype(member(temp))>;
             // Does the map contain the key and hold the correct type?
-            if (! value.count(get_display_name(member)) || 
+            if (! value.count(get_display_name(member)) ||
                 ! std::holds_alternative<member_type>(value.at(get_display_name(member))))
                 result = false;
         }
@@ -186,7 +197,7 @@ constexpr uint8_t pmtTypeIndex()
         return 5;
     else if constexpr (std::same_as<T, std::string>)
         return 6;
-    else if constexpr (std::same_as<T, std::map<std::string, pmt>>)
+    else if constexpr (std::same_as<T, std::map<std::string, pmt, std::less<>>>)
         return 7;
     else if constexpr (std::same_as<T, std::vector<std::string>>)
         return 8;
@@ -277,7 +288,7 @@ std::streamsize _serialize(std::streambuf& sb, const T& arg) {
         // Send length then value
         sz = value.size();
         length += sb.sputn(reinterpret_cast<const char*>(&sz), sizeof(uint64_t));
-        length += sb.sputn(value.data(), value.size());
+        length += sb.sputn(value.data(), static_cast<std::streamsize>(value.size()));
     }
     return length;
 }
@@ -461,18 +472,18 @@ T _deserialize_val(std::streambuf& sb)
         for (size_t i = 0; i < val.size(); i++) {
             sb.sgetn(reinterpret_cast<char*>(&sz), sizeof(uint64_t));
             val[i].resize(sz);
-            sb.sgetn(val[i].data(), sz);
+            sb.sgetn(val[i].data(), static_cast<std::streamsize>(sz));
         }
         return val;
     }
     else if constexpr (PmtMap<T>) {
         map_t val;
 
         uint32_t nkeys;
-        sb.sgetn(reinterpret_cast<char*>(&nkeys), sizeof(nkeys));
+        sb.sgetn(reinterpret_cast<char*>(&nkeys), static_cast<std::streamsize>(sizeof(nkeys)));
         for (uint32_t n = 0; n < nkeys; n++) {
             uint32_t ksize;
-            sb.sgetn(reinterpret_cast<char*>(&ksize), sizeof(ksize));
+            sb.sgetn(reinterpret_cast<char*>(&ksize), static_cast<std::streamsize>(sizeof(ksize)));
             std::vector<char> data;
             data.resize(ksize);
             sb.sgetn(data.data(), ksize);
@@ -529,7 +540,7 @@ T cast(const P& value)
                 }
             }
             // else if constexpr (PmtMap<T> && PmtMap<U>) {
-            //     return std::get<std::map<std::string, pmt_var_t>>(arg);
+            //     return std::get<std::map<std::string, pmt_var_t, std::less<>>>(arg);
             // }
             else
                 throw std::runtime_error(fmt::format(
@@ -583,7 +594,7 @@ struct formatter<P>
     template <typename FormatContext>
     auto format(const P& value, FormatContext& ctx) const {
         // Due to an issue with the c++ spec that has since been resolved, we have to do something
-        // funky here.  See 
+        // funky here.  See
         // https://stackoverflow.com/questions/37526366/nested-constexpr-function-calls-before-definition-in-a-constant-expression-con
         // This problem only appears to occur in gcc 11 in certain optimization modes. The problem
         // occurs when we want to format a vector<pmt>.  Ideally, we can write something like:

diff --git a/include/pmtv/type_helpers.hpp b/include/pmtv/type_helpers.hpp
@@ -23,7 +23,7 @@ struct as_pmt {
                              std::string,
                              std::vector<std::string>,
                              std::vector<rva::self_t>,
-                             std::map<std::string, rva::self_t>
+                             std::map<std::string, rva::self_t, std::less<>>
                              >;
 };
 
@@ -85,7 +85,7 @@ concept UniformStringVector =
     std::ranges::range<T> && std::same_as<typename T::value_type, std::string>;
 
 template <typename T>
-concept PmtMap = std::is_same_v<T, std::map<std::string, pmt_var_t>>;
+concept PmtMap = std::is_same_v<T, std::map<std::string, pmt_var_t, std::less<>>>;
 
 template <typename T>
 concept String = std::is_same_v<T, std::string>;

diff --git a/python/pmtv/bindings/pmt_python.cc b/python/pmtv/bindings/pmt_python.cc
@@ -8,6 +8,12 @@
  */
 
 /* This file is automatically generated using bindtool */
+#ifdef __GNUC__
+#pragma GCC diagnostic push // ignore warning of external libraries that from this lib-context we do not have any control over
+#pragma GCC diagnostic ignored "-Wall"
+#pragma GCC diagnostic ignored "-Wuseless-cast"
+#pragma GCC diagnostic ignored "-Wold-style-cast"
+#endif
 
 #include <pybind11/complex.h>
 #include <pybind11/numpy.h>
@@ -17,6 +23,10 @@
 #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
 #include <numpy/arrayobject.h>
 
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 namespace py = pybind11;
 
 #include <pmtv/pmt.hpp>
@@ -47,8 +57,17 @@ py::object create_numpy_scalar(T val)
 {
     // usage requires initialized NumPy C-API (call _import_array() before use)
     py::object dt = py::dtype::of<T>();
+#ifdef __GNUC__
+#pragma GCC diagnostic push // ignore warning of external libraries that from this lib-context we do not have any control over
+#pragma GCC diagnostic ignored "-Wall"
+#pragma GCC diagnostic ignored "-Wuseless-cast"
+#pragma GCC diagnostic ignored "-Wold-style-cast"
+#endif
     PyObject* scal =
         PyArray_Scalar(&val, reinterpret_cast<PyArray_Descr*>(dt.ptr()), py::int_(sizeof(T)).ptr());
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
     return py::reinterpret_steal<py::object>(scal);
 }
 
@@ -184,7 +203,7 @@ void bind_pmt(py::module& m)
         //     return pmtv::pmt(); }), py::arg{}.noconvert())
         // Map constructor
         .def(py::init(
-            [](const std::map<std::string, pmtv::pmt>& mm) { return pmtv::pmt(mm); }))
+            [](const pmtv::map_t& mm) { return pmtv::pmt(mm); }))
 
         // Fallback for types not directly mapped to pybind types
         // For supporting e.g. numpy.float32 scalar

diff --git a/python/pmtv/bindings/pmtv_python.cc b/python/pmtv/bindings/pmtv_python.cc
@@ -8,11 +8,21 @@
  *
  */
 
+#ifdef __GNUC__
+#pragma GCC diagnostic push // ignore warning of external libraries that from this lib-context we do not have any control over
+#pragma GCC diagnostic ignored "-Wuseless-cast"
+#pragma GCC diagnostic ignored "-Wold-style-cast"
+#endif
+
 #include <pybind11/pybind11.h>
 
 #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
 #include <numpy/arrayobject.h>
 
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 namespace py = pybind11;
 
 void bind_pmt(py::module&);

diff --git a/test/qa_map.cpp b/test/qa_map.cpp
@@ -7,6 +7,8 @@
  */
 #include <gtest/gtest.h>
 #include <complex>
+#include <string>
+#include <string_view>
 
 #include <pmtv/pmt.hpp>
 
@@ -18,6 +20,23 @@ TEST(PmtMap, EmptyMap)
     auto v = get_map(empty);
     v["abc"] = pmt(uint64_t(4));
     v["xyz"] = pmt(std::vector<double>{ 1, 2, 3, 4, 5 });
+
+    using namespace std::literals;
+    using namespace std::string_literals;
+    using namespace std::string_view_literals;
+    auto keyStringLiteral = "abc";
+    std::string keyString = keyStringLiteral;
+    std::string_view keyStringView = keyStringLiteral;
+    EXPECT_TRUE(std::get<uint64_t>(v.at(keyString)) == uint64_t(4));
+    EXPECT_TRUE(std::get<uint64_t >(v.find(keyString)->second) == uint64_t(4));
+    EXPECT_TRUE(std::get<uint64_t>(v.find(keyStringView)->second) == uint64_t(4));
+    EXPECT_TRUE(std::get<uint64_t>(v.find(keyStringLiteral)->second) == uint64_t(4));
+    EXPECT_TRUE(std::get<uint64_t>(v.at("abc"s)) == uint64_t(4));
+    EXPECT_TRUE(std::get<uint64_t>(v.at(keyStringLiteral)) == uint64_t(4));
+    // EXPECT_TRUE(v.at("abc"sv) == pmt(uint64_t(4))) -- not allowed by the ISO-C++ standard
+    // map::at(T key) checks for exact key and its type 'T'
+    // wrapping of std::string_view with std::string is required by ISO-C++ design of map::at(..)
+    EXPECT_TRUE(v.at(std::string(keyStringView)) == pmt(uint64_t(4))); // <- this is OK
 }
 
 
@@ -27,7 +46,7 @@ TEST(PmtMap, PmtMapTests)
     std::vector<int32_t> val2{ 44, 34563, -255729, 4402 };
 
     // Create the PMT map
-    std::map<std::string, pmt> input_map({
+    pmtv::map_t input_map({
         { "key1", val1 },
         { "key2", val2 },
     });
@@ -72,13 +91,13 @@ TEST(PmtMap, get_as)
     std::vector<int32_t> val2{ 44, 34563, -255729, 4402 };
 
     // Create the PMT map
-    std::map<std::string, pmt> input_map({
+    pmtv::map_t input_map({
         { "key1", val1 },
         { "key2", val2 },
     });
     auto x = pmt(input_map);
     // Make sure that we can get the value back out
-    // auto y = std::map<std::string, pmt>(x);
+    // auto y = std::map<std::string, pmt, std::less<>>(x);
     auto y = get_map(x);
     EXPECT_EQ(x == y, true);
 
@@ -92,7 +111,7 @@ TEST(PmtMap, base64)
     std::vector<int32_t> val2{ 44, 34563, -255729, 4402 };
 
     // Create the PMT map
-    std::map<std::string, pmt> input_map({
+    pmtv::map_t input_map({
         { "key1", val1 },
         { "key2", val2 },
     });
@@ -111,7 +130,7 @@ TEST(PmtMap, fmt)
     std::vector<int32_t> val2{ 44, 34563, -255729, 4402 };
 
     // Create the PMT map
-    std::map<std::string, pmt> input_map({
+    pmtv::map_t input_map({
         { "key1", val1 },
         { "key2", val2 },
     });

diff --git a/test/qa_uniform_vector.cpp b/test/qa_uniform_vector.cpp
@@ -221,7 +221,7 @@ TYPED_TEST(PmtVectorFixture, get_as)
     // else
     //     EXPECT_THROW(std::vector<int>(x), ConversionError);
 
-    // using mtype = std::map<std::string, pmt>;
+    // using mtype = std::map<std::string, pmt, std::less<>>;
     // EXPECT_THROW(mtype(x), ConversionError);
 }