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