[Development] How to port from Q_FOREACH to range-based for

[Lars Knoll]

> On 10 Jun 2019, at 13:48, Giuseppe D'Angelo via Development <development at qt-project.org> wrote:
> 
> (Changing the subject to be on topic)
> 
> On 10/06/2019 13:27, Konstantin Tokarev wrote:
>>> At the cost of saying for the 100th time, before this stuff ends up
>>> indexed by Google: you can port away from Q_FOREACH in an automated way
>>> only in trivial cases.*NOT*  in the general case.
>> How is one supposed to port uses of Q_FOREACH in 1559 places then?
> 
> I've wrote it below ↓
> 
>>> For the general case you've only got the 5 minutes solution (c&p&rename
>>> Q_FOREACH into your codebase; assuming the worst case, of lack of
>>> "Qt5Support" or so) or the long walk of checking all usages.
> 
>> What things should be checked during that "long walk", except aforemention measures
>> to avoid detaching? If we cannot have porting tool, we need at least comprehensive
>> porting guide (and if such guide is formalized, I'm pretty sure it will be possible to
>> automate it, at least partially)
> 
> For instance:
> 
> * that in the body of the loop you're not mutating the container through the loop variable itself (before: it would affect a copy; now: it affects the original; solution: take a copy explicitly);
> 
> * that in the body of the loop you're not mutating the container in a way that invalidates iterators (and/or possibly references into it).
> 
> 
> Here's the catch: from the body you may be calling a function which DOES the above mutations. For instance, you're in a member function, iterating over some container held as member variable, and from within the loop call another member function which mutates that container.
> 
> 
> Again, before someone says "who in their minds does this?", _Qt itself_ did it.
> 
> 
> The conclusion is that either such a porting tool will be incomplete and/or incorrect:
> 
> * it may give up as soon as it sees a non-local call (= 99.9% of the loops), making the tool incomplete (and probably useless);
> 
> * or can do some escape/shape analysis to make it more useful, but still incomplete (who's going to implement this?);
> 
> * or it can ignore the problem and refactor away, at the risk of introducing bugs (= incorrect).
> 
> 
> Complete and correct requires solving the halting problem.

So, is removing it worth all the hassle to us and our users? Q_FOREACH is a macro and it doesn’t really cost us anything to keep it around. Yes, it has issues with non Qt containers and I wouldn’t recommend it for any new code. But maybe we could simply fix that part, but making Q_FOREACH emit a compiler warning if used on a container that’s not implicitly shared?

Cheers,
Lars