Skip to content

Commit

Permalink
Clearer error for ambiguous char conversion.
Browse files Browse the repository at this point in the history
Fixes #481.

There is no string conversion for `char`, `signed char`, and `unsigned char`.
For me, those conversions would just be too ambiguous to be safe: should we
convert them as very short strings, or as very small integers?

The one problem was that when application code does try to convert one of
these types (e.g. because the compiler typedefs `int8_t` as `signed char`!)
it fails with a very obscure error message.  People interpret it as a
missing specialisation.

So, ensure that it at least fails with an error that mentions the root
problem: there was an attempt to perform an ambiguous, and disallowed,
conversion.
  • Loading branch information
jtv committed Sep 9, 2021
1 parent 6da3af0 commit 4b3d89f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Use `[[likely]]` & `[[unlikely]]` only in C++20, to silence warnings.
- Fix clang "not a const expression" error in Windows. (#472)
- Fix warnings about `[[likely]]` in `if constexpr`. (#475)
- Clearer error for ambiguous string conversion of `char` type. (#481)
7.6.0
- Removed bad string conversion to `std::basic_string_view<std::byte>`. (#463)
- Add C++20 concepts: `binary`, `char_string`, `char_strings`.
Expand Down
53 changes: 53 additions & 0 deletions include/pqxx/internal/conversions.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,28 @@ inline std::string state_buffer_overrun(HAVE have_bytes, NEED need_bytes)
throw_null_conversion(std::string const &type);


/// Deliberately nonfunctional conversion traits for @c char types.
/** There are no string conversions for @c char and its signed and unsigned
* variants. Such a conversion would be dangerously ambiguous: should we treat
* it as text, or as a small integer? It'd be an open invitation for bugs.
*
* But the error message when you get this wrong is very cryptic. So, we
* derive dummy @c string_traits implementations from this dummy type, and
* ensure that the compiler disallows their use. The compiler error message
* will at least contain a hint of the root of the problem.
*/
template<typename CHAR_TYPE> struct disallowed_ambiguous_char_conversion
{
static char *into_buf(char *, char *, CHAR_TYPE) = delete;
static constexpr zview
to_buf(char *, char *, CHAR_TYPE const &) noexcept = delete;

static constexpr std::size_t
size_buffer(CHAR_TYPE const &) noexcept = delete;
static CHAR_TYPE from_string(std::string_view) = delete;
};


template<typename T> PQXX_LIBEXPORT extern std::string to_string_float(T);


Expand Down Expand Up @@ -218,6 +240,37 @@ template<> struct string_traits<bool>
};


/// We don't support conversion to/from @c char types.
/** Why are these disallowed? Because they are ambiguous. It's not inherently
* clear whether we should treat values of these types as text or as small
* integers. Either choice would lead to bugs.
*/
template<>
struct string_traits<char>
: internal::disallowed_ambiguous_char_conversion<char>
{};

/// We don't support conversion to/from @c char types.
/** Why are these disallowed? Because they are ambiguous. It's not inherently
* clear whether we should treat values of these types as text or as small
* integers. Either choice would lead to bugs.
*/
template<>
struct string_traits<signed char>
: internal::disallowed_ambiguous_char_conversion<signed char>
{};

/// We don't support conversion to/from @c char types.
/** Why are these disallowed? Because they are ambiguous. It's not inherently
* clear whether we should treat values of these types as text or as small
* integers. Either choice would lead to bugs.
*/
template<>
struct string_traits<unsigned char>
: internal::disallowed_ambiguous_char_conversion<unsigned char>
{};


template<> inline constexpr bool is_unquoted_safe<bool>{true};


Expand Down

0 comments on commit 4b3d89f

Please sign in to comment.