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

fix: mark fmt::streamed() as constexpr #3650

Merged
merged 1 commit into from
Sep 19, 2023
Merged

fix: mark fmt::streamed() as constexpr #3650

merged 1 commit into from
Sep 19, 2023

Conversation

muggenhor
Copy link
Contributor

@muggenhor muggenhor commented Sep 19, 2023

Because it's just performing a very basic type conversion that can be done at constexpr time.

My use case simultaneously creates a
fmt::basic_format_string<some_type_conversion<Args...>> instance and performs maybe_stream<Args>(args).... maybe_stream optionally applies fmt::streamed(arg) to a subset of types. This needs to be constexpr because basic_format_string's constructor is consteval.

Reduced example below. Note that I could make both those functions consteval if and only if fmt::streamed is constexpr.

template <typename T>
constexpr decltype(auto) maybe_stream(const T& value) {
  if constexpr (some_condition_v<T>) {
    return fmt::streamed(value);
    // NOTE: currently doing this instead as a workaround:
    return decltype(fmt::streamed(value)){value};
  } else {
    return value;
  }
}

template <
    typename S
  , typename... Args
  , std::enable_if_t<std::is_convertible<const S&, fmt::string_view>::value>* = nullptr
  >
constexpr auto make_log_record(
    ExtraArgs            extra_args
  , const S&             format
  , Args&&...            args
) noexcept {
  using record_t         = log_record_impl<decltype(maybe_stream(std::forward<Args>(args)))...>;
  using format_string_t = typename record_t::format_string_t;
  using args_t          = typename record_t::args_t;

  return record_t{
      extra_args,
      format_string_t(format), // <-- requires std::is_constant_evaluated()==true but won't (always) be if 'maybe_stream' below isn't
      args_t{maybe_stream(std::forward<Args>(args))...},
  };
}

Because it's just performing a very basic type conversion that can be
done at constexpr time.

My use case simultaneously creates a
`fmt::basic_format_string<some_type_conversion<Args...>>` instance and
performs `some_type_conversion<Args>(args)...`. `some_type_conversion`
optionally applies `fmt::streamed(arg)` to a subset of types. This needs
to be `constexpr` because `basic_format_string`'s constructor is
`consteval`.
@vitaut vitaut merged commit a3a74fa into fmtlib:master Sep 19, 2023
40 checks passed
@vitaut
Copy link
Contributor

vitaut commented Sep 19, 2023

Looks reasonable, merged.

@muggenhor muggenhor deleted the fix/constexpr-stream-conversion branch September 20, 2023 07:34
ckerr pushed a commit to transmission/fmt that referenced this pull request Nov 7, 2023
Because it's just performing a very basic type conversion that can be
done at constexpr time.

My use case simultaneously creates a
`fmt::basic_format_string<some_type_conversion<Args...>>` instance and
performs `some_type_conversion<Args>(args)...`. `some_type_conversion`
optionally applies `fmt::streamed(arg)` to a subset of types. This needs
to be `constexpr` because `basic_format_string`'s constructor is
`consteval`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants