Open jkeenan opened 3 years ago
atoomic
commented
3 years ago the work for providing warnings by default has been initiated in alpha-dev-02-strict+warnings branch
once alpha-dev-02-strict is merged we can use this work to create alpha-dev-03-warnings
The relevant commit is 61acf32e3b785ca5b1fe09c1203fc20cc910f438 other commits provide fixup
jkeenan
commented
3 years ago alpha-dev-02-strict+warnings
@atoomic, once I merged and tagged the "02a" branch into alpha, I forked a new branch alpha-dev-03-warnings. I then checked out alpha-dev-02-strict+warnings and, in preparation for merging it into alpha-dev-03-warnings, attempted to rebase it on the new branch.
At this point I got merge conflicts in `pp_ctl.c. This is not surprising, as this was one of the files we touched while working on Objective 2a. But I'd rather not attempt to (once again) resolve merge conflicts in this file (and possibly other files that have been recently touched).
Could I ask that, in the alpha-dev-03-warnings branch, you manually do the changes needed in the guts code to turn on warnings by default? (I.e., do it without a git merge.)
After that, we can go to work on getting test files to run clean with warnings turned on by default.
Thank you very much. Jim Keenan
atoomic
commented
3 years ago @jkeenan I pushed 6564eb081db575d3595355a33168c018fa095de9 to the alpha-dev-03-warnings branch to enable warnings by default outside -e/-E one-liners.
I've also added some extra commits to start fixing warnings in various files. As you would notice a lot of place now needs some attention/love.
jkeenan
commented
3 years ago Over the past few weeks I have been poking at getting test files to run under warnings-by-default. This work is being done in the alpha-dev-03-warnings branch, or in branches forked therefrom. Some directories (e.g., t/run/) now run warnings-free in that branch. There are currently 3 pull requests pending for merger into that branch.
But that still leaves much work to be done until make test in the core distribution runs warnings-free. I have opened issue tickets for each of 9 subdirectories where test files either still emit warnings and/or fail when warnings are turned on by default. When we clear those warnings and get all tests to PASS, we will be able to merge alpha-dev-03-warnings into alpha and declare Objective 3 achieved.
To get a crude idea of how much work remains to be done, we can count the lines of output for each subdirectory during make test when run first in the alpha branch (i.e., before turning on warnings-by-default) and then running make test at HEAD in the alpha-dev-03-warnings branch. The excess is the approximate number of lines of warnings output which we have to address for each subdirectory.
Subdirectory alpha alpha-dev-03-warnings
cpan 1372 1401
dist 440 544
ext 241 316
lib 80 6659
t-comp 25 120
t-lib 10 339
t-op 210 3839
t-opbasic 5 5312
t-re 77 810The following file shows the current status of the alpha-dev-03-warnings branch, which is the current target branch for work on this objective.
http://thenceforward.net/perl/perl7/freebsd.threaded.b8e81cf509.alpha-dev-03-warnings.make-test.output.txt.gz
There are 3 pull requests pending which will further reduce the list of tests still emitting warnings:
t/op/*.tt/re/*.tt/lib/*.t
Most of the work remaining lies in lib/*.t and, to a lesser extent, in ext/ and `dist/'Thank you very much. Jim Keenan
Objective: Implement 'warnings-by-default'.
Modify the guts such that all programs run against a Perl 7 executable run with warnings turned on by default. The statement
use warning;should become a "no-op" except where a particular file has previously called some variant ofno warnings;.No other guts changes except those needed to implement this objective.
Initially, there will be many files in the test suite that will emit warnings once we have warnings turned on by default. How we handle these warnings will depend on the situations in each case. Broadly speaking, we can handle warnings in one of three ways:
If the warning is pointing to something that is truly problematic about the code in the test file, then we should fix the test file to eliminate the problem being warned about.
If the warning is being emitted in a particular instance is important, then we should capture the warning, (e.g., via
$SIG{__WARN__}) and then write unit tests for either the content of the warning and/or the count of warnings emitted.If the warning is of lesser importance (like, say, many "uninitialized value" warnings), then we can quiet the warning with code like:
However, such suppression of warnings should be confined to a narrow scope around the statement which is generating warnings.
Examples: This three-way division of ways of handling warnings is illustrated in the slides beginning here.
Some files in the test suite may already been brought into compliance with both warnings-by-default by the application of cherry-picked code in the preceding objective, strict-by-default. We will also be able to cherry-pick commits from work in earlier branches that fixes problems addressed in other objectives still to come. For example, we have already revised many test files to avoid indirect object syntax. There is no big harm in cherry-picking such commits, provided that we do it judiciously.
Acceptance Criteria: The changes needed to have Perl run with warnings turned on by default are clearly available from the commit history and documented in the commit log. The Perl test suite runs GREEN in all major configurations and on all major operating systems for which we can test. Running the Perl test suite with Perl 7 semantics displays no more warnings than it would when run against a Perl 5 executable in the same configuration and on the same platform.
Note: This objective will probably require more work than any other. We should strongly consider recruiting other Perl developers to help us with this work. After this objective is met, the remaining objectives should be much easier to meet.