Fix ExifTool: eval STRING AST mutation, join LIST context, interpreter improvements#258
Merged
Conversation
…RPRETER opt-out The bytecode interpreter for eval STRING is now enabled by default, providing 46x faster compilation for workloads with many unique eval strings. Set JPERL_EVAL_NO_INTERPRETER=1 to disable. Update error_messages.t to accept interpreter-mode error variations. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
…in LIST context Two fixes for ExifTool test failures: 1. eval STRING was mutating shared AST nodes (operatorAst.id), causing `my` variable declarations inside loops to stop reinitializing after the first eval. Use IdentityHashMap lookup instead of AST mutation. 2. join() with LIST argument was compiling elements in scalar context instead of list context, causing array arguments to return count instead of elements. Also propagate disassemble flag to require/do for debugging. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
The bytecode compiler always treated goto LABEL as non-local control flow (CREATE_GOTO + RETURN), causing Can't find label errors when goto was used within the same function. This is the root cause of ExifTool WriteExif failures -- WriteExif.pl uses goto NoWrite to jump within a loop body across if/elsif branches. Now the bytecode compiler tracks label PCs (gotoLabelPcs map) and emits direct GOTO instructions for intra-function jumps, with forward reference patching for labels not yet seen. This matches the JVM backend behavior. ExifTool Writer.t: 33/61 -> 43/61 (10 more tests passing) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
Replace direct ast.id mutation with IdentityHashMap lookup in all four locations that assign BEGIN variable IDs (SpecialBlockParser, SubroutineParser, BytecodeCompiler, EmitVariable). This prevents eval STRING from corrupting shared AST nodes, which caused my variables in loops to stop reinitializing. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
fglock
force-pushed
the
fix-exiftool-next
branch
from
3a91393 to
84589ad
Compare
UtimeOperator was passing filenames with embedded NUL directly to native utimes(), causing flaky behavior when the NUL-truncated path happened to exist. Use RuntimeIO.sanitizePathname() to strip trailing NULs and reject embedded NULs with a warning, matching the behavior already used by open, glob, and unlink. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
fglock
force-pushed
the
fix-exiftool-next
branch
from
5715091 to
d0a59dd
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
This PR fixes several issues discovered while running the Image::ExifTool test suite, building on the previous ExifTool crash fixes (#256, #257):
myvariables in loops —RuntimeCode.javanow usesIdentityHashMapinstead of mutatingoperatorAst.id, preventing AST corruption when eval is called repeatedly in loops (both JVM and interpreter paths)CompileBinaryOperator.javanow setsLISTcontext for ListNode elements incompileJoinBinaryOp(), fixing cases where join didn't properly flatten listsJPERL_EVAL_NO_INTERPRETERopt-out env varTest plan
makepasses (build + unit tests)make test-allpassesKnown remaining issues (to be addressed in follow-up)
goto LABELnot supported in interpreter (affects WriteExif.pl'sgoto NoWritepattern)Generated with Devin