Skip to content

Commit

Permalink
QString::arg(): don't ignore signedness in (unscoped) enums
Browse files Browse the repository at this point in the history
std::is_signed doesn't work on enums. Instead of SFINAE'ing out for
non-arithmetic types, it returns false. This changes the output of
(unscoped) enums with signed underlying_type from signed to unsigned,
compared to Qt 6.8.

To fix, use an enum's underlying_type to determine signedness. This is
complicated by the fact that std::underlying_type SFINAE's out (since
C++20) or even constitutes UB (until C++17) when called on non-enums,
so we have to be careful to limit the use of it to just enums, which
we reach by passing the meta-function (underlying_type), not its
result (underlying_type_t) to std::conditional and then calling he
result. This requires the non-enum branch to be a meta-function, too
(and not just a type), which is easily achieved using
q20::type_identity.

Use ::type::type instead of mixing _t and ::type to not confuse users
(the leading `typename` is required, anyway, because at least one
trailing ::type is required, and typename std::conditional_t looks
wrong).

Amends 563ed82.

Fixes: QTBUG-131906
Change-Id: I6d122d5a48bffb1e09eb0d7841bb8f1f79cd882f
Reviewed-by: Ivan Solovev <[email protected]>
Reviewed-by: Fabian Kosmale <[email protected]>
  • Loading branch information
marcmutz committed Dec 5, 2024
1 parent d7c9619 commit 2021d39
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/corelib/text/qstring.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,12 @@ class Q_CORE_EXPORT QString
[[nodiscard]] QString arg(T a, int fieldWidth = 0, int base = 10,
QChar fillChar = u' ') const
{
if constexpr (std::is_signed_v<T>)
using U = typename std::conditional<
// underlying_type_t<non-enum> is UB in C++17/SFINAE in C++20, so wrap:
std::is_enum_v<T>, std::underlying_type<T>,
q20::type_identity<T>
>::type::type;
if constexpr (std::is_signed_v<U>)
return arg_impl(qlonglong(a), fieldWidth, base, fillChar);
else
return arg_impl(qulonglong(a), fieldWidth, base, fillChar);
Expand Down
1 change: 0 additions & 1 deletion tests/auto/corelib/text/qstring/tst_qstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6697,7 +6697,6 @@ void tst_QString::arg()
// (unscoped) enums
enum : int { FooS = -1 };
enum : uint { FooU = 1 };
QEXPECT_FAIL("", "QTBUG-131906", Continue); // Qt 6.9 only
QCOMPARE(s4.arg(FooS), QLatin1String("[-1]"));
QCOMPARE(s4.arg(FooU), QLatin1String("[1]"));

Expand Down

0 comments on commit 2021d39

Please sign in to comment.