deprecate the ':all' feature bundle #22817
Conversation
As discussed in Perl/PPCs#60, `:all` is very unlikely to be useful. Since it will enable all features a future Perl might support, it will also enable some that might be incompatible with the code as written (see `bitwise`, for example, which changes the meaning of the existing bitwise operators). With the addition of "negative" features in v5.32, `:all` became even less useful, since it would re-enable features deemed undesirable in modern Perl. As this point in time, `:all` is effectively a footgun. If the keyword `all` (from PPC0027) is added as a feature, there's an extra risk of confusion between `use feature 'all'` and `use feature ':all'`. This patch makes `:all` warn. We should consider removing that bundle entirely in the future.
|
I believe this should be added as a deprecated:: category warning and listed in perldeprecation. But I am obviously in favor. |
|
The feature docs should also mention the deprecation. |
| @@ -1175,6 +1175,8 @@ sub __common { | |||
| my $name = shift; | |||
| if (substr($name, 0, 1) eq ":") { | |||
| my $v = substr($name, 1); | |||
| carp('Feature bundle ":all" is deprecated and should not be used') | |||
There was a problem hiding this comment.
Use
warnings::warnif(deprecated => "...");so it's conditional on the category
There was a problem hiding this comment.
I used carp because the rest of the module used croak for errors. Is there the equivalent for carp? Or should I just not bother?
| sub carp { | ||
| require Carp; | ||
| Carp::carp(@_); | ||
| } | ||
|
|
There was a problem hiding this comment.
This can be removed by using warnings::warnif
|
There are modules on CPAN that do https://grep.metacpan.org/search?q=use+feature.*%3Aall&qd=&qft=&qifl= |
|
Not many hits (so far?); primarily looks like one author's test file and another author that should really be warned not to use such an unpredictable feature set which also keeps misfeatures around. I'm curious if anyone has ever used no feature ":all". |
|
https://grep.metacpan.org/search?size=20&_bb=443602641&q=no+feature.*%3Aall&qd=&qft=&qifl= finds one actual hit on CPAN for no feature ":all" in this test file: https://metacpan.org/release/OALDERS/TOML-Tiny-0.18/source/t/toml-test/valid/array/array.t - none of which do anything because they are immediately followed by using a real feature bundle. |
|
And another interesting case is in App::EvalServerAdvanced's deparse handler, which removes such a construct from deparse output, something that might be worth double checking. |
Actually, this is useful.
ensures only the 5.36 bundle features are enabled, just as |
|
The code in TOML::Tiny is horrible and should be fixed regardless of what happens with this ticket. But the reason it has |
|
Perhaps the warning should only be printed by |
This seems like an odd inconsistency. |
Something like |
As discussed in Perl/PPCs#60,
:allis very unlikely to be useful.Since it will enable all features a future Perl might support, it will also enable some that might be incompatible with the code as written (see
bitwise, for example, which changes the meaning of the existing bitwise operators).With the addition of "negative" features in v5.32,
:allbecame even less useful, since it would re-enable features deemed undesirable in modern Perl.As this point in time,
:allis effectively a footgun.If the keyword
all(from PPC0027) is added as a feature, there's an extra risk of confusion betweenuse feature 'all'anduse feature ':all'.This patch makes
:allwarn. We should consider removing that bundle entirely in the future.