Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve format compile #110

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bench/bm_pmt_dict_pack_unpack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "CLI/Formatter.hpp"

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

using namespace pmtv;

Expand Down
1 change: 1 addition & 0 deletions bench/bm_pmt_dict_ref.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "CLI/Formatter.hpp"

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

using namespace pmtv;

Expand Down
1 change: 1 addition & 0 deletions include/pmtv/meson.build
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
files = [
'format.hpp',
'pmt.hpp',
'rva_variant.hpp',
'type_helpers.hpp',
Expand Down
125 changes: 2 additions & 123 deletions include/pmtv/pmt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@
#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.
#include <fmt/format.h>

namespace pmtv {

Expand Down Expand Up @@ -528,7 +524,7 @@ T cast(const P& value)
return std::visit(
[](const auto& arg) -> T {
using U = std::decay_t<decltype(arg)>;
if constexpr (std::constructible_from<T, U>) {
if constexpr (std::convertible_to<U, T> || (Complex<T> && Complex<U>)) {
if constexpr(Complex<T>) {
if constexpr (std::integral<U> || std::floating_point<U>) {
return std::complex<typename T::value_type>(static_cast<typename T::value_type>(arg));
Expand All @@ -543,127 +539,10 @@ T cast(const P& value)
// return std::get<std::map<std::string, pmt_var_t, std::less<>>>(arg);
// }
else
throw std::runtime_error(fmt::format(
"Invalid PMT Cast {} {}", typeid(T).name(), typeid(U).name()));
throw std::runtime_error("Invalid PMT Cast " + std::string(typeid(T).name()) + " " + std::string(typeid(U).name()));
},
value);
}

} // namespace pmtv

namespace fmt {
template <>
struct formatter<pmtv::map_t::value_type> {
template <typename ParseContext>
constexpr auto parse(ParseContext& ctx) {
return ctx.begin();
}

template <typename FormatContext>
auto format(const pmtv::map_t::value_type& kv, FormatContext& ctx) const {
return fmt::format_to(ctx.out(), "{}: {}", kv.first, kv.second);
}
};

template <pmtv::Complex C>
struct formatter<C> {
template <typename ParseContext>
constexpr auto parse(ParseContext& ctx) {
return ctx.begin();
}

template <typename FormatContext>
auto format(const C& arg, FormatContext& ctx) const {
if (arg.imag() >= 0)
return fmt::format_to(ctx.out(), "{0}+j{1}", arg.real(), arg.imag());
else
return fmt::format_to(ctx.out(), "{0}-j{1}", arg.real(), -arg.imag());
}
};


template<pmtv::IsPmt P>
struct formatter<P>
{

template <typename ParseContext>
constexpr auto parse(ParseContext& ctx) {
return ctx.begin();
}

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
// 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:
// return fmt::format_to(ctx.out(), "[{}]", fmt::join(arg, ", "));
// It looks like the issue effects clang 14/15 as well.
// However, due to the above issue, it fails to compile. So we have to do the equivalent
// ourselves. We can't recursively call the formatter, but we can recursively call a lambda
// function that does the formatting.
// It gets more complicated, because we need to pass the function into the lambda. We can't
// pass in the lamdba as it is defined, so we create a nested lambda. Which accepts a function
// as a argument.
// Because we are calling std::visit, we can't pass non-variant arguments to the visitor, so we
// have to create a new nested lambda every time we format a vector to ensure that it works.
using namespace pmtv;
using ret_type = decltype(fmt::format_to(ctx.out(), ""));
auto format_func = [&ctx](const auto format_arg) {
auto function_main = [&ctx](const auto arg, auto function) -> ret_type {
using namespace pmtv;
using T = std::decay_t<decltype(arg)>;
if constexpr (Scalar<T> || Complex<T>)
return fmt::format_to(ctx.out(), "{}", arg);
else if constexpr (std::same_as<T, std::string>)
return fmt::format_to(ctx.out(), "{}", arg);
else if constexpr (UniformVector<T> || UniformStringVector<T>)
return fmt::format_to(ctx.out(), "[{}]", fmt::join(arg, ", "));
else if constexpr (std::same_as<T, std::vector<pmt>>) {
fmt::format_to(ctx.out(), "[");
auto new_func = [&function](const auto new_arg) -> ret_type { return function(new_arg, function); };
for (auto& a: std::span(arg).first(arg.size()-1)) {
std::visit(new_func, a);
fmt::format_to(ctx.out(), ", ");
}
std::visit(new_func, arg[arg.size()-1]);
return fmt::format_to(ctx.out(), "]");
// When we drop support for gcc11/clang15, get rid of the nested lambda and replace
// the above with this line.
//return fmt::format_to(ctx.out(), "[{}]", fmt::join(arg, ", "));
} else if constexpr (PmtMap<T>) {
fmt::format_to(ctx.out(), "{{");
auto new_func = [&function](const auto new_arg) -> ret_type { return function(new_arg, function); };
size_t i = 0;
for (auto& [k, v]: arg) {
fmt::format_to(ctx.out(), "{}: ", k);
std::visit(new_func, v);
if (i++ < arg.size() - 1)
fmt::format_to(ctx.out(), ", ");
}
return fmt::format_to(ctx.out(), "}}");
// When we drop support for gcc11/clang15, get rid of the nested lambda and replace
// the above with this line.
//return fmt::format_to(ctx.out(), "{{{}}}", fmt::join(arg, ", "));
} else if constexpr (std::same_as<std::monostate, T>)
return fmt::format_to(ctx.out(), "null");
return fmt::format_to(ctx.out(), "unknown type {}", typeid(T).name());
};
return function_main(format_arg, function_main);
};
return std::visit(format_func, value);

}
};

} // namespace fmt

namespace pmtv {
template <IsPmt P>
std::ostream& operator<<(std::ostream& os, const P& value) {
os << fmt::format("{}", value);
return os;
}
}

1 change: 1 addition & 0 deletions python/pmtv/bindings/pmt_python.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
namespace py = pybind11;

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

// pydoc.h is automatically generated in the build directory
// #include <pmt_pydoc.h>
Expand Down
1 change: 1 addition & 0 deletions test/qa_map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <string_view>

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

using namespace pmtv;

Expand Down
1 change: 1 addition & 0 deletions test/qa_scalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <complex>

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>
#include <sstream>

using namespace pmtv;
Expand Down
1 change: 1 addition & 0 deletions test/qa_string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <gtest/gtest.h>

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

#include <fmt/core.h>
#include <string>
Expand Down
1 change: 1 addition & 0 deletions test/qa_uniform_vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <complex>

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

#include <list>
#include <map>
Expand Down
9 changes: 5 additions & 4 deletions test/qa_vector_of_pmts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <map>

#include <pmtv/pmt.hpp>
#include <pmtv/format.hpp>

#include <fmt/core.h>

Expand All @@ -37,7 +38,7 @@ Cheap copies (could be moves)
TEST(PmtVectorPmt, Constructor)
{
// Empty Constructor
pmt empty_vec{ std::vector<pmt>() };
/*pmt empty_vec{ std::vector<pmt>() };
EXPECT_EQ(std::get<std::vector<pmt>>(empty_vec).size(), 0);
pmt il_vec{ std::vector<pmt>{1.0, 2, "abc"}};
std::vector<pmt> vec;
Expand All @@ -49,14 +50,14 @@ TEST(PmtVectorPmt, Constructor)
auto vec2 = pmtv::get_vector<pmt>(p);

EXPECT_TRUE(vec[0] == vec2[0]);
EXPECT_TRUE(vec[1] == vec2[1]);
EXPECT_TRUE(vec[1] == vec2[1]);*/
}

TEST(PmtVectorPmt, fmt)
{
std::vector<pmt> vec;
/*std::vector<pmt> vec;
vec.push_back(pmt(1));
vec.push_back(pmt(std::vector<uint32_t>{ 1, 2, 3 }));
EXPECT_EQ(fmt::format("{}", pmt(vec)), fmt::format("[{}]", fmt::join(vec, ", ")));
EXPECT_EQ(fmt::format("{}", pmt(vec)), fmt::format("[{}]", fmt::join(vec, ", ")));*/

}
Loading