Deprecate smartmatch

[book]

This is a follow-up from http://nntp.perl.org/group/perl.perl5.porters/264678.

This pull request replaces experimental::smartmatch by deprecated::smartmatch.
The goal is to be more explicit that smartmatch is a failed experiment, and its
use is discouraged.

This also remove some uses of ~~ in the core code.

[Grinnz]

The changes to autodie and to the Test-Simple and experimental.pm tests have to be done on CPAN.

[leonerd]

This also remove some uses of ~~ in the core code.

I wonder if it would be best to do those first in a separate commit PR. That way, even if someone objects to this overall change, it would still be good to get those uses out of core code anyhow. Those parts at least don't seem draft-worthy so those can be a real PR we can review and merge, ahead of this one.

[Leont]

I don't think we should do this until we have a CPAN module that can largely drop-in replace the core functionality (I don't think it even has to be 100%).

I think it's entirely feasible to do this for given/when (I mean, it would largely just be adopting a few chunks code from core). Doing the same for ~~ would require @leonerd 's custom infix operator branch first.

[book]

The changes to autodie and to the Test-Simple and experimental.pm tests have to be done on CPAN.

I know. I made the changes in the branch to try and get the test suite to pass.

The current version of the branch has one test failing, and I don't understand why:

$ ./perl harness ../lib/warnings.t 
../lib/warnings.t .. 781/932 FILE: lib/warnings/toke ; line 1743
PROG: 
BEGIN { binmode STDERR, ":utf8" }
use utf8;
use feature 'extra_paired_delimiters';
no warnings 'experimental::extra_paired_delimiters';
my $good = q<this string uses ASCII delimiter; no warning>;
my $good2 = q«this string has mirrored delimiters in either order»;
my $good3 = q»and reversed«;
no feature 'extra_paired_delimiters';
my $warn3 = q«this string starts and ends with the lhs terminator«;
my $warn4 = q»this string starts and ends with the rhs mirror»;
EXPECTED:
Use of '«' is deprecated as a string delimiter at - line 9.
Use of '»' is deprecated as a string delimiter at - line 10.
GOT:

# Failed test 887 - extra paired delimiters Latin1 range in UTF-8 at lib/warnings/toke line 1743
FILE: lib/warnings/toke ; line 1758
PROG: 
BEGIN { binmode STDERR, ":utf8" }
use utf8;
use feature 'extra_paired_delimiters';
no warnings 'experimental::extra_paired_delimiters';
my $good = q<this string uses ASCII delimiter; no warning>;
my $good2 = q《this string has a mirrored terminator》;
my $warn2 = q》this string starts and ends with the rhs mirror》;
my $good3 = q‹this string has mirrored delimiters in either order›;
my $good3 = q›and reversed‹;
no feature 'extra_paired_delimiters';
my $warn3 = q《this string starts and ends with the lhs terminator《;
my $warn4 = q》this string starts and ends with the rhs mirror》;
my $warn5 = q‹this string starts and ends with the lhs terminator‹;
my $warn6 = q›this string starts and ends with the rhs mirror›;
EXPECTED:
Use of '》' is deprecated as a string delimiter at - line 7.
Use of '《' is deprecated as a string delimiter at - line 11.
Use of '》' is deprecated as a string delimiter at - line 12.
Use of '‹' is deprecated as a string delimiter at - line 13.
Use of '›' is deprecated as a string delimiter at - line 14.
GOT:

# Failed test 888 - extra paired delimiters above Latin1 range at lib/warnings/toke line 1758
#
# Note: 'run_multiple_progs' run has one or more failures
#        you can consider setting the environment variable
#        PERL_TEST_ABORT_FIRST_FAILURE=1 before running the test
#        to stop on the first error.
#
../lib/warnings.t .. Failed 2/932 subtests 
        (less 1 skipped subtest: 929 okay)

Test Summary Report
-------------------
../lib/warnings.t (Wstat: 0 Tests: 932 Failed: 2)
  Failed tests:  887-888
Files=1, Tests=932,  3 wallclock secs ( 0.08 usr  0.02 sys +  2.48 cusr  0.64 csys =  3.22 CPU)
Result: FAIL

[book]

I don't think we should do this until we have a CPAN module that can largely drop-in replace the core functionality (I don't think it even has to be 100%).

The gist of this PR is basically s/experimental::smartmatch/deprecated::smartmatch/. The feature is still there and working, but the warning to disable is slightly different, and is more explicit about our position about using smartmatch.

The changes to autodie are mostly to disable the correct warning depending on the version of Perl.
The changes to Test-Simple are probably too simplistic.
I thought I'd submit them upstream once this PR was ready to merge (to get the version numbers right).

The actual removal can be done at a later stage (if ever).

[jkeenan]

An observation and some questions:

CI is reporting a porting test failure, which I reproduced locally. I believe it can be fixed with ./perl -Ilib regen/warnings.pl.

1. Is this the first time we're proposing use of the deprecated:: namespace (syntax?) in the core distribution? Was it discussed in an RFC or somewhere else? (I'm not raising an objection, just seeking information.)

In blead, I don't see it anywhere.

$ gitcurr
blead
$ ack -l 'deprecated::' .
$ 

2. If so, I'm a bit surprised not to find that string anywhere in the "guts" (*.c or *.h files).

$ ack -l 'deprecated::' . 2>/dev/null
t/op/tie_fetch_count.t
t/op/state.t
t/op/cmpchain.t
t/op/taint.t
t/op/coreamp.t
t/op/switch.t
t/op/smartmatch.t
t/lib/croak/pp_ctl
t/lib/warnings/9uninit
t/lib/warnings/op
t/lib/feature/switch
lib/B/Deparse.t
lib/warnings.pm
lib/overload.t
regen/warnings.pl
dist/Safe/t/safeops.t
cpan/experimental/t/basic.t
cpan/Test-Simple/t/Legacy/Regression/870-experimental-warnings.t
cpan/autodie/t/exceptions.t
cpan/autodie/lib/Fatal.pm
pod/perldiag.pod
ext/XS-APItest/t/grok.t

Where is its "core" implementation?

[leonerd]

1. Is this the first time we're proposing use of the `deprecated::` namespace (syntax?) in the core distribution?  Was it discussed in an RFC or somewhere else?  (I'm not raising an objection, just seeking information.)

I don't think it was discussed very deeply anywhere. We originally tried just using the normal deprecated warning but that's too blunt an instrument - if someone does want to silence it for whatever reason, they'll end up silencing all the deprecated warnings. Having its own category avoids that trouble.

In blead, I don't see it anywhere.

...
Indeed, it's new.

2. If so, I'm a bit surprised not to find that string anywhere in the "guts" (`*.c` or `*.h` files).

It won't be anywhere currently.

[book]

CI is reporting a porting test failure, which I reproduced locally. I believe it can be fixed with ./perl -Ilib regen/warnings.pl.

Yup, I forgot to rerun it after my last modification and rebase. Expect another force push soon.

1. Is this the first time we're proposing use of the deprecated:: namespace (syntax?) in the core distribution? Was it discussed in an RFC or somewhere else? (I'm not raising an objection, just seeking information.)

We discussed this briefly in a PSC meeting. @leonerd said something about having wanted it in the past, but never actually doing it. The deprecated warning currently encompasses all deprecations.

It was said on p5p that some were knowingly using smartmatch in a controlled way, knowing its flaws. (I haven't been able to find that email back, so I might have imagined it?) We felt that it would make sense to make it a bit more fine-grained than simply having it covered by the deprecated warning. We don't want people to silence all deprecation warnings just because they want to keep using smartmatch while they still can.

The relevant p5p threads are:

• http://nntp.perl.org/group/perl.perl5.porters/264087
• http://nntp.perl.org/group/perl.perl5.porters/264678

Creating the deprecated::smartmatch is our way of saying more clearly that smartmatch is a failed experiment (and that I may be removed in a future version of Perl). It replaces experimental::smartmatch (so no warnings 'experimental::smartmatch'; effectively becomes a no-op).

This has the interesting side effect that the deprecated category is the only category where enabling all its leaves enables less than the category itself. I had to patch regen/warnings.pl (with @leonerd's help) to forcefully keep deprecated enabled by default, now that it is a category.

In blead, I don't see it anywhere.

$ git grep WARN_DEPRECATED
dist/Devel-PPPort/parts/apidoc.fnc:Amnhd||WARN_DEPRECATED
dist/Devel-PPPort/parts/base/5006000:WARN_DEPRECATED                # E
dist/Devel-PPPort/parts/inc/warn:__UNDEFINED__  WARN_DEPRECATED          2
dist/Devel-PPPort/parts/todo/5003007:WARN_DEPRECATED                # T
handy.h:#  define deprecate(s) Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),    \
handy.h:              Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),    \
handy.h:              Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),    \
pp_ctl.c:            bool do_warn = namesv && ckWARN_d(WARN_DEPRECATED)
pp_ctl.c:                Perl_warner(aTHX_ packWARN(WARN_DEPRECATED),
regcomp.c:    _WARN_HELPER(loc, packWARN(WARN_DEPRECATED),                        \
regcomp.c:                      Perl_warner(aTHX_ packWARN(WARN_DEPRECATED),      \
regcomp.c:    _WARN_HELPER(loc, packWARN(WARN_DEPRECATED),                        \
regcomp.c:                      Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED), \
regcomp.c:    _WARN_HELPER(loc, packWARN2(WARN_DEPRECATED, WARN_REGEXP),              \
regcomp.c:                      Perl_ck_warner_d(aTHX_ packWARN2(WARN_DEPRECATED,     \
regcomp.c:        Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
t/porting/diag.t:        $category .= ',WARN_DEPRECATED';
toke.c:            Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
utf8.c:    if (ckWARN_d(WARN_DEPRECATED)) {
utf8.c:                Perl_warner(aTHX_ WARN_DEPRECATED,
utf8.c:                Perl_warner(aTHX_ WARN_DEPRECATED,
warnings.h:#define WARN_DEPRECATED                       2
warnings.h:=for apidoc Amnh||WARN_DEPRECATED

[Leont]

The gist of this PR is basically s/experimental::smartmatch/deprecated::smartmatch/. The feature is still there and working, but the warning to disable is slightly different, and is more explicit about our position about using smartmatch.

You say deprecated, but do you actually mean deprecated or just discouraged? I don't think it's a good idea for experimental.pm to disable deprecated warnings, it would be fine to disable discouraged ones as long as it's well-documented to do so.

[book]

You say deprecated, but do you actually mean deprecated or just discouraged?

We actually mean "deprecated". And this clearly says that the next step (at a yet undetermined point in the future) is to remove smartmatch entirely.

I don't think it's a good idea for experimental.pm to disable deprecated warnings, it would be fine to disable discouraged ones as long as it's well-documented to do so.

I agree with you: use experimental 'smartmatch'; shouldn't imply no warnings 'deprecated::smartmatch';. A deprecation warning is much stronger than an experimental one, and should be knowningly disabled.

Which means my patch against experimental.pm can be dropped entirely, which I'll do in my next push. (I might need to update the test, though.)

[Grinnz]

I agree; anyone who is using use experimental 'smartmatch'; should begin receiving deprecation warnings to be aware of the change in status, and they can then disable that warning if they choose.

[book]

I think the next steps are:

• get the top three commits in the branch on their respective upstream repositories,
• pull the changes back in blead
• merge the branch

[Leont]

The actual removal can be done at a later stage (if ever).

But that does mean end-users will have to change their code twice to keep it working. That's not very end-user friendly.

[book]

It's more about giving users ample warning that smartmatch is going away.

Changing experimental::smartmatch to deprecated::smartmatch is a simple mechanical change.

It's a bit more difficult to change code using smartmatch to code not using it; on the other hand, that's a change one can make as soon as they see the deprecation warning.

So, best case, they only need to change their code once, to remove smartmatch as soon as we announce it's deprecated.

[haarg]

This seems to be carving out a special type of deprecation that isn't really deprecation, because we don't plan on actually removing it.

If we want to deprecate it, lets just do it normally, without a special category. If we aren't willing to commit to removing it, it should be stabilized.

[toddr]

autodie 2.35 is on CPAN and I have submitted a sync here. If you have any issues, please let me know and we can sort it!

#20746

[toddr]

I have merged to blead. You should be able to rebase and no longer need to modify autodie

[demerphq]

I rebased and resolved conflicts on this.

[book]

I rebased and resolved conflicts on this.

Thanks. I believe this is ready to be merged, now that the dual-life modules have all been updated.

regen/feature.pl has been re-run since

[demerphq]

LGTM

[demerphq]

FWIW, some folks would say we are in the contentious code freeze period. I personally would disagree, and waiting another release cycle would not be helpful.

[iabyn]

On Fri, Feb 24, 2023 at 08:49:39AM -0800, Yves Orton wrote: Merged #20337 into blead.

cpan/autodie/t/exceptions.t is producing a bunch of warnings. Wasn't this supposed to be been sorted by pulling in autodie 2.36? $ ./perl -Ilib cpan/autodie/t/exceptions.t Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 25. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 26. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 27. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 28. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 45. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 46. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 47. Smartmatch is deprecated at cpan/autodie/t/exceptions.t line 48. ok 1 - Exception thrown ...

…

-- In England there is a special word which means the last sunshine of the summer. That word is "spring".

[demerphq]

Wasn't this supposed to be been sorted by pulling in autodie 2.36?

@toddr

[demerphq]

@iabyn @toddr - I have gotten to the bottom of it, PR coming up. The issue is that @toddr assumed that "deprecated::smartmatch" was a valid warning category. But until my PR is pushed and merged we actually dont support this at all, and dont even have a way to, as we only have a single category "deprecated" and the data structure doesnt support us turning it on AND having a sub category. I have a fix in hand already

[book]

The initial PR added a deprecated::smartmatch category. Later, after some discussions within the PSC, we agreed that having subcategories for deprecated wasn't really useful, and possibly counter-productive.

[toddr]

I have gotten to the bottom of it, PR coming up. The issue is that @toddr assumed that "deprecated::smartmatch" was a valid warning category.

I didn't really assume anything about deprecated::smartmatch. But I guess I took the change with the assumption it was safe in 5.36 and safe in blead as you'd be merging changes soon. @demerphq can you clarify what we need to change in autodie at this point? PRs welcome at https://github.com/pjf/autodie/pulls

[haarg]

The problem with autodie is not really that it is using the wrong category. The problem is that it uses and tests for smartmatch. It needs more extensive changes. I've begun working on fixing it.

[haarg]

I've prepared Dual-Life/autodie#117 which adjusts autodie to use the correct warning category, but also to deprecate its own smartmatch handling.

[toddr]

I've prepared pjf/autodie#117 which adjusts autodie to use the correct warning category, but also to deprecate its own smartmatch handling.

Ok. So I don't jump the gun, is it ready for release? have we solidified how the perl side will work?