Front page | perl.perl5.porters |
Postings from April 2017
Re: [perl #130497] Revert "Unescaped left brace in regex" fatality
Thread Previous
|
Thread Next
From:
Karl Williamson
Date:
April 10, 2017 04:20
Subject:
Re: [perl #130497] Revert "Unescaped left brace in regex" fatality
Message ID:
efafa169-40fe-ed48-04d6-acab13d1c8c1@khwilliamson.com
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.
>
> (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