Front page | perl.perl5.porters |
Postings from April 2017
Re: [perl #130497] Revert "Unescaped left brace in regex" fatality
Thread Previous
|
Thread Next
From:
Sawyer X
Date:
April 12, 2017 09:04
Subject:
Re: [perl #130497] Revert "Unescaped left brace in regex" fatality
Message ID:
76a1e387-799a-a392-3d64-ee31489801e7@gmail.com
On 04/10/2017 06:20 AM, Karl Williamson wrote:
> 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 <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?
>
> 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.
Fair enough.
(I have read dutifully, I just don't have much to add and I don't want
to just hash it.)
>>
>> (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.
I misread when you wrote 5.30 above. Added another stable and got 32.
Did I mention I'm dyslexic? :)
>>
>> 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.
We had already released another dev for the @INC change. Several have
asked for another dev release in order to clear out more kinks in CPAN.
>
>>
>> [1] This is a real word, I checked.
>>
>
> Don't misunderestimate.
<3
Thread Previous
|
Thread Next