D26129: Remove Iterator based loops for range based loops
Kevin Ottens
noreply at phabricator.kde.org
Mon Dec 23 20:44:19 GMT 2019
ervin added a comment.
In D26129#582392 <https://phabricator.kde.org/D26129#582392>, @tcanabrava wrote:
> I didn't blindly change anything, everything I did I tested either by running the unittests or recompilling and testing the settings of applications.
Sure I know you did this, otherwise I think I'd call you criminal. ;-)
More seriously, I find the test suite of kconfig_compiler unfortunately very slim on error cases (as in XML with crap in them) so that's why we should try to keep in mind "creative" users during reviews. As a corollary, the absence of failing tests doesn't mean the absence of bugs being introduced.
As I'm trying to highlight in my comment below, I think that in that particular case moving away from iterators introduce a regression in behavior and information provided to the user in case of bogus data.
INLINE COMMENTS
> tcanabrava wrote in kconfig_compiler.cpp:2119
> if two items in the list have the same values the KConfig XML is wrong and the generated code will not compile. I don't see an issue with it.
Well, there's a difference between generating malformed code and generating well formed code which doesn't compile for grammar reasons.
Previously it wouldn't compile with a proper error (two parameters with the same name) now it would in most case give a totally unrelated error since the tokenizer would go haywire.
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/5def1cc2/attachment.html>
More information about the Kde-frameworks-devel
mailing list