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

Question about narrow/wide string conversion #690

Closed
sebkoenig opened this issue Mar 21, 2018 · 4 comments
Closed

Question about narrow/wide string conversion #690

sebkoenig opened this issue Mar 21, 2018 · 4 comments

Comments

@sebkoenig
Copy link
Contributor

Is there a documentation about which char type conversions are supposed to work and which should not work by design?

Narrow string to wide string output seems to work:
fmt::format(L"{}", "foo");

Wide char to narrow string seems to be explictly unsupported with a static_assert
fmt::format("{}", L'f');

formatting of wide characters into a narrow output is disallowed

Wide string to narrow string compiles, but puts the memory address of the wide string into the output, which doesn't look like intended behavior?
fmt::format("{}", L"foo");

"0x7ff6f817dc80"

Then there seem to be compile error tests in fmt/test/compile-test/CmakeLists.txt which look broken:

# Formatting a wide string with a narrow format string is forbidden.
expect_compile_error("fmt::format(\"{}\", L\"foo\";")

Looks like a missing ) for the function call?

Can someone please explain why (or correct me if I am wrong) narrow to wide is supported, but wide to narrow should not work?
Thanks!

@vitaut
Copy link
Contributor

vitaut commented Mar 22, 2018

Is there a documentation about which char type conversions are supposed to work and which should not work by design?

One case is mentioned in http://fmtlib.net/latest/index.html#safety, but otherwise I don't think it's documented, just enforced by compile-time checks.

Combining multi-byte (narrow) and wide strings is prohibited for reasons explained in #606.

Narrow string to wide string output seems to work

It shouldn't compile in the master branch: https://godbolt.org/g/JVA8MZ.

Wide string to narrow string compiles

This shouldn't compile either: https://godbolt.org/g/WqaYrN

Looks like a missing ) for the function call?

Good catch, thanks!

@sebkoenig
Copy link
Contributor Author

Ok I found the issue.
narrow to wide, and wide to narrow compile as soon as you include <fmt/ostream.h>. See https://godbolt.org/g/SsRFiF

For wide to narrow fmt::format("{}", L"foo"); , the function

template <bool IS_PACKED, typename Context, typename T>
inline typename std::enable_if<IS_PACKED, value<Context>>::type
    make_arg(const T &value) {
  return make_value<Context>(value);
}

will call the make_arg overload generated by the macro:
FMT_MAKE_VALUE(pointer_type, const void*, const void*)
So my wide string gets formatted as a void*.

For narrow to wide fmt::format(L"{}", "foo");, this make_value overload gets used:

template <typename C, typename T, typename Char = typename C::char_type>
inline typename std::enable_if<
    !convert_to_int<T, Char>::value &&
    !std::is_convertible<T, basic_string_view<Char>>::value &&
    !std::is_convertible<T, std::basic_string<Char>>::value,
    typed_value<C, custom_type>>::type
  make_value(const T &val) { return val; }

which then handles the narrow string as a custom type, which leads to this format_value call:

template <typename Char, typename T>
void format_value(basic_buffer<Char> &buffer, const T &value) {
  internal::FormatBuf<Char> format_buf(buffer);
  std::basic_ostream<Char> output(&format_buf);
  output.exceptions(std::ios_base::failbit | std::ios_base::badbit);
  output << value;
  buffer.resize(buffer.size());
}

with Char = wchar_t, T = char[3]

@vitaut
Copy link
Contributor

vitaut commented Mar 27, 2018

Including fmt/ostream.h disables some of the safety checks with the undesirable effects that you've observed. I'll try to improve the checks making them work with ostreams.

compile-test should be fixed now.

@vitaut
Copy link
Contributor

vitaut commented Mar 30, 2018

Fixed in e8e006f. Now all the cases except formatting a narrow char into a wide string are disallowed. Thanks for reporting!

@vitaut vitaut closed this as completed Mar 30, 2018
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

No branches or pull requests

2 participants