Skip to content

Commit

Permalink
Move formatting code to a different header file to improve compile times
Browse files Browse the repository at this point in the history
Signed-off-by: John Sallay <jasallay@gmail.com>
  • Loading branch information
jsallay committed Apr 27, 2024
1 parent 6b8a886 commit 252b082
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 125 deletions.
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
120 changes: 120 additions & 0 deletions include/pmtv/format.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// 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 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 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
127 changes: 2 additions & 125 deletions include/pmtv/pmt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +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 {

using pmt = pmt_var_t;
Expand Down Expand Up @@ -528,7 +523,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 +538,9 @@ 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 @@ -10,6 +10,7 @@
#include <pmtv/pmt.hpp>

#include <fmt/core.h>
#include <pmtv/format.hpp>
#include <string>

using namespace pmtv;
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
1 change: 1 addition & 0 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 Down

0 comments on commit 252b082

Please sign in to comment.