atoomic / perl

a repo to show what could be p7
Other
18 stars 8 forks source link

ext/: Remove 'use strict' #104

Closed jkeenan closed 3 years ago

jkeenan commented 4 years ago

There are 235 files underneath ext/ in which the string use strict; can be found.

[perlmonger: perl-atoomic] $ ack -l 'use strict;' ext/ | wc -l
     235

Files under ext/ are maintained in blead. They are not separately released to CPAN. (Or, at least, they should not be. If they are separately released, they should be in dist/.)

In Perl 7, use strict; will be on by default. That should mean that that string can be removed -- correct?

The list:

[perlmonger: perl-atoomic] $ ack -l 'use strict;' ext/
ext/Pod-Functions/Makefile.PL
ext/Pod-Functions/t/Functions.t
ext/Pod-Functions/Functions_pm.PL
ext/B/Makefile.PL
ext/B/t/xref.t
ext/B/t/o.t
ext/B/t/walkoptree.t
ext/B/t/optree_concise.t
ext/B/t/showlex.t
ext/B/t/perlstring.t
ext/B/t/concise.t
ext/B/t/optree_check.t
ext/B/t/OptreeCheck.pm
ext/B/t/pragma.t
ext/B/t/b.t
ext/B/t/strict.t
ext/B/B/Xref.pm
ext/B/B/Showlex.pm
ext/B/B/Concise.pm
ext/B/B/Terse.pm
ext/Hash-Util/t/Util.t
ext/Hash-Util/t/builtin.t
ext/Hash-Util/lib/Hash/Util.pm
ext/Tie-Memoize/lib/Tie/Memoize.pm
ext/Tie-Memoize/t/Tie-Memoize.t
ext/XS-APItest/APItest.pm
ext/XS-APItest/t/temp_lv_sub.t
ext/XS-APItest/t/handy00.t
ext/XS-APItest/t/overload.t
ext/XS-APItest/t/utf8_warn08.t
ext/XS-APItest/t/copyhints.t
ext/XS-APItest/t/peep.t
ext/XS-APItest/t/my_cxt.t
ext/XS-APItest/t/blockhooks-csc.t
ext/XS-APItest/t/underscore_length.t
ext/XS-APItest/t/keyword_multiline.t
ext/XS-APItest/t/utf8_warn02.t
ext/XS-APItest/t/rv2cv_op_cv.t
ext/XS-APItest/t/callregexec.t
ext/XS-APItest/t/magic.t
ext/XS-APItest/t/postinc.t
ext/XS-APItest/t/op.t
ext/XS-APItest/t/hash.t
ext/XS-APItest/t/arrayexpr.t
ext/XS-APItest/t/cleanup.t
ext/XS-APItest/t/utf8_warn03.t
ext/XS-APItest/t/swaptwostmts.t
ext/XS-APItest/t/keyword_plugin.t
ext/XS-APItest/t/stuff_modify_bug.t
ext/XS-APItest/t/cophh.t
ext/XS-APItest/t/synthetic_scope.t
ext/XS-APItest/t/utf8_warn09.t
ext/XS-APItest/t/autoload.t
ext/XS-APItest/t/handy01.t
ext/XS-APItest/t/call_checker.t
ext/XS-APItest/t/handy03.t
ext/XS-APItest/t/load-module.t
ext/XS-APItest/t/gv_const_sv.t
ext/XS-APItest/t/op_list.t
ext/XS-APItest/t/utf8_to_bytes.t
ext/XS-APItest/t/join_with_space.t
ext/XS-APItest/t/utf8_warn01.t
ext/XS-APItest/t/handy09.t
ext/XS-APItest/t/addissub.t
ext/XS-APItest/t/grok.t
ext/XS-APItest/t/stmtasexpr.t
ext/XS-APItest/t/op_contextualize.t
ext/XS-APItest/t/handy08.t
ext/XS-APItest/t/utf8_warn00.t
ext/XS-APItest/t/xsub_h.t
ext/XS-APItest/t/sviscow.t
ext/XS-APItest/t/whichsig.t
ext/XS-APItest/t/gotosub.t
ext/XS-APItest/t/savehints.t
ext/XS-APItest/t/handy02.t
ext/XS-APItest/t/pmflag.t
ext/XS-APItest/t/handy06.t
ext/XS-APItest/t/blockhooks.t
ext/XS-APItest/t/looprest.t
ext/XS-APItest/t/rmagical.t
ext/XS-APItest/t/handy_base.pl
ext/XS-APItest/t/lvalue.t
ext/XS-APItest/t/swaplabel.t
ext/XS-APItest/t/svpeek.t
ext/XS-APItest/t/utf8_warn04.t
ext/XS-APItest/t/stuff_svcur_bug.t
ext/XS-APItest/t/ptr_table.t
ext/XS-APItest/t/multicall.t
ext/XS-APItest/t/newCONSTSUB.t
ext/XS-APItest/t/refs.t
ext/XS-APItest/t/call.t
ext/XS-APItest/t/gv_fetchmethod_flags.t
ext/XS-APItest/t/gv_autoload4.t
ext/XS-APItest/t/pad_scalar.t
ext/XS-APItest/t/hv_macro.t
ext/XS-APItest/t/caller.t
ext/XS-APItest/t/utf8_warn05.t
ext/XS-APItest/t/fetch_pad_names.t
ext/XS-APItest/t/bootstrap.t
ext/XS-APItest/t/sort.t
ext/XS-APItest/t/xs_special_subs.t
ext/XS-APItest/t/subsignature.t
ext/XS-APItest/t/utf8.t
ext/XS-APItest/t/handy07.t
ext/XS-APItest/t/scopelessblock.t
ext/XS-APItest/t/handy05.t
ext/XS-APItest/t/customop.t
ext/XS-APItest/t/newDEFSVOP.t
ext/XS-APItest/t/win32.t
ext/XS-APItest/t/utf16_to_utf8.t
ext/XS-APItest/t/gv_init.t
ext/XS-APItest/t/gv_fetchmeth.t
ext/XS-APItest/t/gv_fetchmeth_autoload.t
ext/XS-APItest/t/utf8_warn07.t
ext/XS-APItest/t/svcatpvf.t
ext/XS-APItest/t/keyword_plugin_threads.t
ext/XS-APItest/t/weaken.t
ext/XS-APItest/t/coplabel.t
ext/XS-APItest/t/stmtsasexpr.t
ext/XS-APItest/t/loopblock.t
ext/XS-APItest/t/eval-filter.t
ext/XS-APItest/t/my_exit.t
ext/XS-APItest/t/blockasexpr.t
ext/XS-APItest/t/utf8_warn06.t
ext/XS-APItest/t/magic_chain.t
ext/XS-APItest/t/clone-with-stack.t
ext/XS-APItest/t/utf8_warn_base.pl
ext/XS-APItest/t/svsetsv.t
ext/XS-APItest/t/xs_special_subs_require.t
ext/XS-APItest/t/labelconst.t
ext/XS-APItest/t/handy04.t
ext/DynaLoader/Makefile.PL
ext/DynaLoader/DynaLoader_pm.PL
ext/DynaLoader/t/DynaLoader.t
ext/re/t/lexical_debug.t
ext/re/t/re_funcs_u.t
ext/re/t/strict.t
ext/re/t/reflags.t
ext/re/t/regop.t
ext/re/t/re.t
ext/re/t/re_funcs.t
ext/re/re.pm
ext/PerlIO-via/t/thread.t
ext/PerlIO-via/t/via.t
ext/SDBM_File/t/constants.t
ext/SDBM_File/t/prep.t
ext/SDBM_File/t/corrupt.t
ext/SDBM_File/SDBM_File.pm
ext/SDBM_File/Makefile.PL
ext/I18N-Langinfo/t/Langinfo.t
ext/I18N-Langinfo/Langinfo.pm
ext/Sys-Hostname/Hostname.pm
ext/ODBM_File/ODBM_File.pm
ext/File-Find/lib/File/Find.pm
ext/File-Find/t/find.t
ext/File-Find/t/lib/Testing.pm
ext/GDBM_File/t/fatal.t
ext/GDBM_File/GDBM_File.pm
ext/File-Glob/t/rt131211.t
ext/File-Glob/t/basic.t
ext/File-Glob/t/threads.t
ext/File-Glob/t/rt114984.t
ext/File-Glob/Glob.pm
ext/ExtUtils-Miniperl/lib/ExtUtils/Miniperl.pm
ext/Pod-Html/lib/Pod/Html.pm
ext/Pod-Html/t/crossref.t
ext/Pod-Html/t/htmlescp.t
ext/Pod-Html/t/htmldir1.t
ext/Pod-Html/t/cache.t
ext/Pod-Html/t/htmlview.t
ext/Pod-Html/t/htmldir3.t
ext/Pod-Html/t/htmldir2.t
ext/Pod-Html/t/htmllink.t
ext/Pod-Html/t/crossref3.t
ext/Pod-Html/t/poderr.t
ext/Pod-Html/t/feature2.t
ext/Pod-Html/t/anchorify.t
ext/Pod-Html/t/crossref2.t
ext/Pod-Html/t/feature.t
ext/Pod-Html/t/htmldir5.t
ext/Pod-Html/t/htmldir4.t
ext/Pod-Html/t/podnoerr.t
ext/Pod-Html/testdir/perlpodspec-copy.pod
ext/VMS-DCLsym/DCLsym.pm
ext/Errno/Errno_pm.PL
ext/NDBM_File/NDBM_File.pm
ext/Hash-Util-FieldHash/lib/Hash/Util/FieldHash.pm
ext/Hash-Util-FieldHash/t/02_function.t
ext/Hash-Util-FieldHash/t/03_class.t
ext/Hash-Util-FieldHash/t/11_hashassign.t
ext/Hash-Util-FieldHash/t/01_load.t
ext/Hash-Util-FieldHash/t/04_thread.t
ext/Hash-Util-FieldHash/t/05_perlhook.t
ext/Hash-Util-FieldHash/t/12_hashwarn.t
ext/File-DosGlob/lib/File/DosGlob.pm
ext/Fcntl/Fcntl.pm
ext/Fcntl/t/syslfs.t
ext/Fcntl/t/autoload.t
ext/mro/mro.pm
ext/XS-Typemap/t/Typemap.t
ext/Opcode/t/Opcode.t
ext/Opcode/Opcode.pm
ext/PerlIO-encoding/t/threads.t
ext/PerlIO-encoding/encoding.pm
ext/IPC-Open3/t/IPC-Open2.t
ext/IPC-Open3/t/IPC-Open3.t
ext/IPC-Open3/t/fd.t
ext/IPC-Open3/lib/IPC/Open2.pm
ext/IPC-Open3/lib/IPC/Open3.pm
ext/POSIX/lib/POSIX.pm
ext/POSIX/t/sysconf.t
ext/POSIX/t/math.t
ext/POSIX/t/sigset.t
ext/POSIX/t/time.t
ext/POSIX/t/waitpid.t
ext/POSIX/t/termios.t
ext/POSIX/t/unimplemented.t
ext/POSIX/t/usage.t
ext/POSIX/t/posix.t
ext/POSIX/t/export.t
ext/POSIX/t/iscrash
ext/POSIX/t/wrappers.t
ext/POSIX/t/sigaction.t
ext/attributes/attributes.pm
ext/PerlIO-scalar/t/scalar_ungetc.t
ext/PerlIO-mmap/mmap.pm
ext/Amiga-Exec/Exec.pm
ext/Amiga-Exec/__examples/simplecommand.pl
ext/Amiga-Exec/__examples/simplehost.pl
ext/Amiga-ARexx/__examples/simplecommand.pl
ext/Amiga-ARexx/__examples/simplehost.pl
ext/Amiga-ARexx/ARexx.pm
ext/FileCache/lib/FileCache.pm
ext/Tie-Hash-NamedCapture/t/tiehash.t
ext/Tie-Hash-NamedCapture/NamedCapture.pm
atoomic commented 4 years ago

short answer: yes we can remove strict from these files longer one: I would prefer to wait a little before removing the useless use strict and use warnings, as I think this will makes our process to submit existing changes to blead easier.

I would do it late in the development process.

jkeenan commented 4 years ago

short answer: yes we can remove strict from these files longer one: I would prefer to wait a little before removing the useless use strict and use warnings, as I think this will makes our process to submit existing changes to blead easier.

I would do it late in the development process.

Fair enough, I did remove use strict from two files I patched this morning, but will refrain from doing so for other files in ext and dist until further discussion.

khwilliamson commented 3 years ago

It seems to me to be make work to remove these lines. I don't understand the motivation

jkeenan commented 3 years ago

It seems to me to be make work to remove these lines. I don't understand the motivation

When we set out to work on this project in June, our objective was (and still largely is) a proof-of-concept, namely, "This is what the Perl core distribution would like if (for example) strict was on by default."

Well, if strict is on by default, you shouldn't need to declare it at the top of any file. You should only need it if, for some reason, you needed to relax strictures at some point in the file and later wanted to enforce them again.

So the clearest possible way to demonstrate this is to strip out use strict; at the top of most files.

Now, as the discussion in this ticket back in July suggests, we later realized that files located in the dist/ and cpan/ directories should retain use strict; because they either originate on CPAN (cpan/) or are actually or potentially releasable to CPAN (dist/). So we've been leaving use strict; in those files even though in the "Perl 7" strict-by-default world such statements are superfluous.

That leaves the files in ext/. They're not released to CPAN (or, at least, they're not supposed to be released to CPAN) so they're completely under the control of the core distribution. So in those files use strict; can be removed. And it's not "make work"; it's pretty much just a one-liner, probably this:

find ext -type f | ack  -i '\.(p[lm]|t)$' | xargs perl -pi -e 's{^use strict;$}{}'

... which I'll take care of tomorrow.

Thank you very much. Jim Keenan

jkeenan commented 3 years ago

See branch jkeenan/gha-104-ext-strict and https://github.com/atoomic/perl/pull/293