Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'-Dusedefaultstrict` is premature; recommend removal #18872

Open
jkeenan opened this issue Jun 9, 2021 · 9 comments
Open

'-Dusedefaultstrict` is premature; recommend removal #18872

jkeenan opened this issue Jun 9, 2021 · 9 comments

Comments

@jkeenan
Copy link
Contributor

jkeenan commented Jun 9, 2021

Inspired by @rjbs's talk at the Perl Conference today, I decided to see what happens when you configure with -Dusedefaultstrict.

$ sh ./Configure -des -Dusedevel -Duseithreads -Dusedefaultstrict

# sanity check
$ grep -n strict config.sh
17:config_args='-des -Dusedevel -Duseithreads -Dusedefaultstrict'
22:config_arg4='-Dusedefaultstrict'
67:ccflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
90:cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
1155:usedefaultstrict='define'

$ make test_harness
[snip]
./miniperl -w -Ilib -Idist/Exporter/lib -MExporter -e '<?>' || sh -c 'echo >&2 Failed to build miniperl.  Please run make minitest; exit 1'
./miniperl -Ilib -f write_buildcustomize.pl
./miniperl -Ilib make_patchnum.pl
Updating 'git_version.h' and 'lib/Config_git.pl'
./miniperl -Ilib configpm
written lib/Config.pod
updated lib/Config.pm
updated lib/Config_heavy.pl
./miniperl -Ilib utils/Makefile.PL
Global symbol "@ISA" requires explicit package name (did you forget to declare "my @ISA"?) at /usr/home/jkeenan/gitwork/perl/cpan/Text-Tabs/lib/Text/Wrap.pm line 6.
Global symbol "@EXPORT" requires explicit package name (did you forget to declare "my @EXPORT"?) at /usr/home/jkeenan/gitwork/perl/cpan/Text-Tabs/lib/Text/Wrap.pm line 7.
Global symbol "@EXPORT_OK" requires explicit package name (did you forget to declare "my @EXPORT_OK"?) at /usr/home/jkeenan/gitwork/perl/cpan/Text-Tabs/lib/Text/Wrap.pm line 8.
Global symbol "$VERSION" requires explicit package name (did you forget to declare "my $VERSION"?) at /usr/home/jkeenan/gitwork/perl/cpan/Text-Tabs/lib/Text/Wrap.pm line 10.
Global symbol "$SUBVERSION" requires explicit package name (did you forget to declare "my $SUBVERSION"?) at /usr/home/jkeenan/gitwork/perl/cpan/Text-Tabs/lib/Text/Wrap.pm line 11.
BEGIN not safe after errors--compilation aborted at /usr/home/jkeenan/gitwork/perl/cpan/Text-Tabs/lib/Text/Wrap.pm line 13.
Compilation failed in require at ./regen/regen_lib.pl line 6.
BEGIN failed--compilation aborted at ./regen/regen_lib.pl line 6.
Compilation failed in require at utils/Makefile.PL line 16.
*** Error code 255

Stop.
make: stopped in /usr/home/jkeenan/gitwork/perl

This looked familiar.

When @atoomic and I were working on Sawyer's "Perl 7" proposal last year, we came to a point where Nico had flipped the bit that turned strictures on by default. We then had to proceed systematically to get make to complete, then make test_prep, and finally the whole test suite with make test_harness.

Several hundred commits and person-hours later ... we achieved that objective. (One then, of course, faces the challenge of getting CPAN code and darkpan code to run with strictures on by default.)

But until such time as we're willing to invest that time and effort, I don't see the point of offering the -Dusedefaultstrict configuration option in a production release of perl. We should remove it from blead and confine it to a branch.

Thank you very much.
Jim Keenan

@neilb
Copy link
Contributor

neilb commented Jun 13, 2021

Let's see if we can get the upstream CPAN distributions all strict-clean first. I'll ask Aristotle to do a release of Text-Tabs+Wrap and if that's good, to then do a PR with his changes.

@Leont
Copy link
Contributor

Leont commented Jun 13, 2021

Let's see if we can get the upstream CPAN distributions all strict-clean first. I'll ask Aristotle to do a release of Text-Tabs+Wrap and if that's good, to then do a PR with his changes.

It will also need to #18662 for make to pass.

The real challenge is making make test pass though.

@neilb
Copy link
Contributor

neilb commented Jun 13, 2021

Update: @ap has said he'll get Text-Tabs+Wrap in good strict/warnings shape in the near, but not immediate, future.

@DrHyde
Copy link
Contributor

DrHyde commented Jun 14, 2021

While I wouldn't dream of turning on usedefaultstrict in production, I do think it would be a very useful thing indeed on one of my CPAN-testing perls.

@jkeenan
Copy link
Contributor Author

jkeenan commented Jun 14, 2021

While I wouldn't dream of turning on usedefaultstrict in production, I do think it would be a very useful thing indeed on one of my CPAN-testing perls.

The problem is that I don't see how we can, in good conscience, ship a version of perl that offers a configuration option that will not currently pass the core distribution's own test suite.

@hvds
Copy link
Contributor

hvds commented Jun 14, 2021

The problem is that I don't see how we can, in good conscience, ship a version of perl that offers a configuration option that will not currently pass the core distribution's own test suite.

I think your conscience should be clear if that fact is clearly documented; with far less concern we regularly ship perls that don't pass all tests on some platforms.

@wolfsage
Copy link
Contributor

wolfsage commented Jun 14, 2021 via email

@ap
Copy link
Contributor

ap commented Jun 14, 2021

I don’t see a problem with shipping the option now even without a big caution sign in the documentation (re @hvds) – that would only be necessary if users were being invited and excited to try the new shiny. But that would be silly. There’s no reason perldelta has to shout it from the rooftops right away. Just ship it low-key in the footnotes as experimental for the curious to play with; only advertise it loudly and to everyone (again) later once it’s known to work – even if it’s not strictly speaking a new feature in that release. IMO.

@ap
Copy link
Contributor

ap commented Jul 17, 2021

Update: @ap has said he'll get Text-Tabs+Wrap in good strict/warnings shape in the near, but not immediate, future.

It’s out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants