Skip to content

Conversation

@marcmutz
Copy link
Contributor

@marcmutz marcmutz commented Aug 7, 2023

Qt is defaulting to QT_NO_FOREACH these days, so make sure we integrate nicely with downstream and fix the single Q_FOREACH/foreach user, in tst_encoder.cpp.

Unfortunately, the container's initialization code doesn't exactly lend itself to making the container const (not even IILE (Immediately-Invoked Lambda Expression) would help here, due to the interdependency with len), so the idiomatic solution would be to use std::as_const()/qAsConst().

The former is available from C++17, which we don't require, yet, and the latter is not available under QT_NO_AS_CONST (the default for Qt these days), so grab the nettle and implement a t17::as_const() that switches between a manual implementation of std::as_const and the real thing, depending on __cpp_lib_as_const. The t17 here mimicks the qNN (q20::remove_cvref_t/q23::forward_like/etc) mechanism used in Qt itself for backports, with s/q/t/ because ... _T_inyCbor.

The t17 implementation is local to tst_encoder.cpp, but can easily be extracted into a separate header once more users emerge.

Qt is defaulting to QT_NO_FOREACH these days, so make sure we
integrate nicely with downstream and fix the single Q_FOREACH/foreach
user, in tst_encoder.cpp.

Unfortunately, the container's initialization code doesn't exactly
lend itself to making the container const (not even IILE
(Immediately-Invoked Lambda Expression) would help here, due to the
interdependency with `len`), so the idiomatic solution would be to use
std::as_const()/qAsConst().

The former is available from C++17, which we don't require, yet, and
the latter is not available under QT_NO_AS_CONST (the default for Qt
these days), so grab the nettle and implement a t17::as_const() that
switches between a manual implementation of std::as_const and the real
thing, depending on __cpp_lib_as_const. The `t17` here mimicks the qNN
(q20::remove_cvref_t/q23::forward_like/etc) mechanism used in Qt
itself for backports, with s/q/t/ because ... _T_inyCbor.

The t17 implementation is local to tst_encoder.cpp, but can easily be
extracted into a separate header once more users emerge.
@thiagomacieira thiagomacieira merged commit aee4f97 into intel:main Aug 7, 2023
@marcmutz marcmutz deleted the Q_FOREACH branch August 7, 2023 16:29
qtprojectorg pushed a commit to qt/qtbase that referenced this pull request Aug 7, 2023
Upstream change: intel/tinycbor#238

Says it in the upstream commit message:

> Unfortunately, the container's initialization code doesn't exactly
> lend itself to making the container const (not even IILE
> (Immediately-Invoked Lambda Expression) would help here, due to the
> interdependency with len), so the idiomatic solution would be to use
> std::as_const()/qAsConst().
>
> The former is available from C++17, which we don't require, yet, and
> the latter is not available under QT_NO_AS_CONST (the default for Qt
> these days), so grab the nettle and implement a t17::as_const() that
> switches between a manual implementation of std::as_const and the
> real thing, depending on __cpp_lib_as_const. The t17 here mimicks
> the qNN (q20::remove_cvref_t/q23::forward_like/etc) mechanism used
> in Qt itself for backports, with s/q/t/ because ... _T_inyCbor.
>
> The t17 implementation is local to tst_encoder.cpp, but can easily
> be extracted into a separate header once more users emerge.

Task-number: QTBUG-115839
Change-Id: I200ae6abbb6173fa9d4b9964be66501c2145f066
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
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