Re: [RFC] Auto-capture multi-line closures and short functions take 2

On Sat, Mar 27, 2021, at 11:05 AM, Rowan Tommins wrote: > On 27/03/2021 00:05, Nuno Maduro wrote: > > > > I've just added a few more tests with the exact examples you have > > presented in this mail list: > > > > https://github.com/php/php-src/pull/6246/commits/c3a50d671c5d8fa4b775ec67fe77d0cbd5cc8030 > > > > > > <https://github.com/php/php-src/pull/6246/commits/c3a50d671c5d8fa4b775ec67fe77d0cbd5cc8030>. > > > Hi Nuno, > > Thanks, I hadn't thought of writing out test cases, that makes a lot of > sense; although I now realise my question wasn't very clear. > > My biggest concern with automatic capture is the potential for > *accidentally* capturing variables - that is, intending to introduce a > local variable inside the closure, but capturing a variable from the > outer scope that happens to have the same name. This is less of an issue > with capture by value, but can still mean resources not being freed, > e.g. a large array of data not being freed from memory, or an object > destructor not executing when expected. > > This is more likely in PHP than many other languages, because there is > no requirement, or even an option, to explicitly declare a local > variable in the closure. It's not a new problem, but since > single-expression closures are unlikely to use many local variables, > it's probably not one that's been given lots of thought. > > > I've written some tests that demonstrate the current behaviour using an > object destructor: > > https://github.com/nunomaduro/php-src/commit/ae18662cc92f5d07520b4574dcae71d38a9e0a41 > > Based on those, there seems to be no way to prevent a variable being > captured, even if its value is immediately discarded each time the > closure runs. This may be less than ideal: > > $results = getLargeReportFromDatabase(); > // ... > $fn = fn() { >     $results = []; // coincidentally the same name, immediately > assigned its own value >     // ... > } > unset($results); // does not free the array, because $fn has captured > the value > > > I wonder if the capture analysis could be made smarter to make this less > likely. > > > PS I'd like to apologise if some of my messages in this thread have come > across as harsh or demanding, I do appreciate you bringing up this > feature, as I know it's one a lot of people want, even if I'm sceptical. > > Regards, It's a valid point that auto-capture on multi-line function makes the potential for unexpected behavior higher than with a single-line function, simply by virtue of there being more lines of code that could do so. The question is whether that higher risk is worth the improved ergonomics.. That is, ultimately, a subjective question, and depends on how much you think the risk increases (it's non-zero, but that doesn't mean high) and how much you think the ergonomics are improved (also non-zero, but hard to quantify). IMO, it is. In practice, I can't recall seeing 100 line closures inside 100 functions, well, ever. I mostly see 2-3 line closures in 10 line functions. That makes the risk vastly smaller, and in practice I don't think it will come up much. Also, bear in mind that in many cases such bugs would become self-evident very quickly, and you'll know to just rename a variable. I realize it's completely and totally unscientific, but the social media response to both of these RFCs has been overwhelmingly (although not universally) positive. There definitely seems to be demand for "do the same stuff but with less typing". --Larry Garfield