D26129: Remove Iterator based loops for range based loops

Kevin Ottens noreply at phabricator.kde.org
Mon Dec 23 17:18:12 GMT 2019


ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.


  The patch being large I didn't check each one separately but I suspect this patch misses boat loads of qAsConst. Besides there's at least one case of very flawed logic, it can't be about blindly switching one loop construct for another especially when the iterators are compared inside the loop body.

INLINE COMMENTS

> kconfig_compiler.cpp:2119
>                  h << type << " " << argument.variableName;
> -                if (++it != itEnd) {
> +                if (argument != signal.arguments.last()) {
>                      h << ", ";

This logic is very flawed... instead of comparing positions in the list now you're comparing actual values. If two different positions in the list would end up having the same values (for some reason) we'd generate syntactically incorrect code.

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D26129

To: tcanabrava, ervin
Cc: ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191223/448dc22b/attachment.html>


More information about the Kde-frameworks-devel mailing list