develooper Front page | perl.perl5.porters | Postings from April 2017

Re: [perl #130497] Revert "Unescaped left brace in regex" fatality

Thread Previous | Thread Next
Karl Williamson
April 10, 2017 04:20
Re: [perl #130497] Revert "Unescaped left brace in regex" fatality
Message ID:
On 04/09/2017 12:01 PM, Sawyer X wrote:
> 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 <> 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?

I don't think you understand the proposal.  The goal is to work around 
autoconf's failure to release a fix in 4 years, so far.  In 5.25, 
autoconf won't compile because a deprecation is now made fatal.  The 
issue is a single pattern in autoconf.  All we would be doing is, if we 
are about to die, to first look to see if it is that exact pattern, and 
if so, warn, don't die.  Thus autoconf would work as it does in 5.24. 
And it would continue to do so as long as it doesn't get upgraded.  As 
soon as it is upgraded, the already-commited fix in its repository would 
cause it to avoid this issue entirely.

Any other code anywhere that had that exact same pattern would also warn 
not die, just as what it does in 5.24.  And there wouldn't be a problem 
with that, as the pattern does not use '{' in a way that would conflict 
with anything currently planned by perl.  So we are removing a "crash", 
not adding one.

"Misrecognizing" something other than this pattern as this pattern isn't 
a problem, because if we did, we give them the same behavior as has 
existed in 5.24 and earlier.

"Misrecognizing" this pattern for something else would cause a problem 
for autoconf.  But we would verify, before release, that it actually did 
solve the autoconf problem.  Remember, we are looking at working around 
a single line in the current version of autoconf; nothing more.
> (If we can move the warning to 5.32, why would the deprecation warning
> be a show-stopper?)

I don't understand the above at all.  I wonder if '32' is a typo.  My 
goal is to get something in 5.28.  We can't have two messages displayed 
with one saying 28, and the other 30.  They would both have to say the 
later release: 30; and that means nothing could get added in 28.
> If we go with the kludge, at the very least it would require having
> another dev release.

I mostly agree, and I thought that was already in the plans because of 
@INC.  But it's not clear to me that attempting a reversion wouldn't 
also require such a release.

> [1] This is a real word, I checked.

Don't misunderestimate.

Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About