On 04/09/2017 04:51 AM, Karl Williamson wrote: > On 04/07/2017 12:21 PM, Kent Fredric wrote: >> On 8 April 2017 at 06:07, Sawyer X <xsawyerx@gmail.com> wrote: >>> Not a bad idea, but we're pretty far in the cycle to test this change >>> out, I think. >> >> >> Yeah, the cleverer the solution, the more time we need to make sure we >> didn't introduce additional bugs in the cleverness. >> >> Which in practice means this late, without additional test cycles, >> this change would be worse than either choice of "leave it in" or >> "take it out". >> > > I think people should be aware of the full consequences of this. > > It is not just a simple matter of reverting this patch. Code has > marched on. In particular, you may recall that I missed some cases > where the deprecation should have been output, and code to output > these was added after this patch, and so presumes that these cases are > fatal. > > If I change the patch to just silently continue, the other code fails > to output a warning on some of the cases covered by this. If I > instead change it to output a warning, some cases will have 2 warnings > output. And cleverness is required to fix that. We could say that at > this stage in development, that we can live with 2 warnings. But the > other warning explicitly says it will be made fatal in 5.30. Part of > the reason to make these cases fatal now, was so that in 5.28, we > could implement some of the extensions made feasible by this. Having > 2 warnings closes the door on that possibility. > > The solution that requires the least cleverness is my original kludge, > to simply make > > /\${[^\}]*}/ > > non-fatal. It's trivial to do this, and keep it to a single warning To be honest, I'm a bit concerned about introducing this kludge. The questions that jump to mind are: * Is there a chance we would misrecognize[1] the context in XS code or core code? (I'm thinking of the oneliner in eval in heredoc in regex s///e and such.) * What happens if we misidentify this context? Will it crash? If it does, will this error message be useful? (If we can move the warning to 5.32, why would the deprecation warning be a show-stopper?) If we go with the kludge, at the very least it would require having another dev release. [1] This is a real word, I checked.Thread Previous | Thread Next