Fix jcpan -t DateTime regressions: XSLoader @ISA fallback and foreach null-stores#443
Merged
Merged
Conversation
fglock
force-pushed
the
fix/jcpan-datetime-regressions
branch
from
31a1d65 to
9ad8b91
Compare
…itCleanup Three issues fixed: 1. XSLoader @isa fallback too broad (from #441): When a module like Clone has @isa=(Exporter) and XSLoader::load fails to find the Java XS class, the fallback code treated any non-empty @isa as proof the module can work through inheritance. This prevented Clone::PP from loading, breaking the entire DateTime dependency chain. Fix: add NON_FUNCTIONAL_ISA set (Exporter, DynaLoader, AutoLoader, XSLoader) and only succeed on @isa fallback when a functional parent is present. 2. scopeExitCleanup closing shared IO handles (from #440): The foreach body null-stores with closeIO=true called scopeExitCleanup on ALL scalars, including copies of shared filehandle references. This broke Test2 TODO mechanism because Test2::Formatter::TAP copies STDOUT handles into a loop variable, and scopeExitCleanup closed them prematurely. Fix: add ioOwner flag to RuntimeScalar, set only by IOOperator.open() when creating a new anonymous glob. scopeExitCleanup now only closes IO on scalars marked as IO owners. Copies via set() do not inherit the flag. 3. TAP::Parser::Iterator::Process warnings: Guard sysread return value against empty string, skip $fh == $err comparison when $err is not a real filehandle (empty string in PerlOnJava), and protect err handle close against missing refs. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
fglock
force-pushed
the
fix/jcpan-datetime-regressions
branch
from
9ad8b91 to
321ca79
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
XSLoader @isa fallback too broad (regression from Template Toolkit support: 105/106 tests passing (99%) #441): When a module like Clone has
@ISA = qw(Exporter)and XSLoader::load fails to find the Java XS class, the fallback code treated any non-empty@ISAas proof the module can work through inheritance. This prevented Clone::PP from loading, breaking the entire DateTime dependency chain. Fixed by adding aNON_FUNCTIONAL_ISAset (Exporter, DynaLoader, AutoLoader, XSLoader) and only succeeding on@ISAfallback when a functional parent is present.emitScopeExitNullStores in foreach body (regression from WWW::Mechanize CPAN module support #440): Nulling JVM local variable slots at the end of each foreach loop iteration broke Test2 TODO mechanism. After a
{ local $TODO = "reason"; ... }block,done_testing()and subsequent test output were suppressed. Fixed by removing the body-scope null-stores from EmitForeach while keeping the outer-scope null-stores which run once after the loop completes.TAP::Parser::Iterator::Process: Guard
sysreadreturn value against empty string and protecterrhandle close against missing refs.Test plan
makepasses all unit tests./jcpan -j 8 -t DateTimepasses all 51 tests (3589/3589 subtests)./jperl -e 'use Test::More; ok(1, "test 1"); { local $TODO = "reason"; ok(1, "passing todo"); } ok(1, "test 3"); done_testing()'outputs all 3 tests and planGenerated with Devin