Skip to content

Remove dead code and hot-path assertion in _check#4

Closed
Koan-Bot wants to merge 12 commits intomasterfrom
koan.atoomic/cleanup-dead-code-and-assertion
Closed

Remove dead code and hot-path assertion in _check#4
Koan-Bot wants to merge 12 commits intomasterfrom
koan.atoomic/cleanup-dead-code-and-assertion

Conversation

@Koan-Bot
Copy link
Copy Markdown
Collaborator

@Koan-Bot Koan-Bot commented Mar 7, 2026

What

Remove unreachable dead code and a debug-only assertion from the production hot path.

Why

  • _check_from_stat() had two unreachable lines (die + return) after an exhaustive if/elsif/else chain where every branch already returns or dies
  • _check() ran die if scalar @others on every mocked file check call, but XS always passes exactly 2 args — this assertion could never trigger and added overhead to the hot path
  • A stale $others[0] reference survived from the old @others parameter — always undef since XS passes 2 args

How

  • Deleted the 3 unreachable lines in _check_from_stat()
  • Changed _check() signature from ($optype, $file, @others) to ($optype, $file)
  • Replaced dead $out // $others[0] with $out
  • Added t/check-args-count.t to verify the XS→Perl 2-argument contract

Testing

Full test suite passes: 51 files, 1073 tests, 0 failures.

🤖 Generated with Claude Code

Koan-Bot and others added 12 commits February 22, 2026 00:53
_check() stored the file argument in $_last_call_for for -X _ caching.
When the argument was a filehandle reference (not a string path), this
prevented the filehandle from being garbage collected, keeping the
underlying file descriptor open.

This caused "spooky action-at-a-distance" bugs: e.g. a socketpair read
hanging because a dup'd write-end filehandle was kept alive by the
leaked reference, even after leaving scope.

Fix: only cache string filenames in $_last_call_for, not references.

Ref: cpan-authors/Test-MockFile#179

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ef-leak

fix: prevent filehandle reference leak in stat override
Add CLAUDE.md with project guidance for Claude Code sessions.
Exclude CLAUDE.md and local/ directory from Dist::Zilla GatherDir
so they don't end up in the CPAN tarball.
- Remove unreachable lines 512-514 in _check_from_stat() after
  exhaustive if/elsif/else chain (every branch returns or dies)
- Remove `die if scalar @others` debug assertion from _check(),
  which runs on every mocked file check. The XS layer always passes
  exactly 2 arguments (optype, file), making this check impossible
  to trigger in production.
- Fix stale `$others[0]` reference on line 626 — @Others was always
  empty since XS passes exactly 2 args, so the fallback was dead code
- Add t/check-args-count.t to verify the XS→Perl arg count contract

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@atoomic
Copy link
Copy Markdown
Owner

atoomic commented Apr 23, 2026

@Koan-Bot squash

@Koan-Bot
Copy link
Copy Markdown
Collaborator Author

Superseded by upstream refactoring: the dead code (unreachable die + return after if/elsif/else chain) was eliminated when _check_from_stat was rewritten as a dispatch table. The stat comment block was extracted into _normalize_stat_result(). Closing.

@Koan-Bot Koan-Bot closed this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants