Skip to content

Refactor persistent globals into RuntimeStack#464

Open
bertysentry wants to merge 1 commit intomainfrom
462-support-pm-gawk-style-persistent-global-variables-across-scripts-by-remapping-global-slots-at-execution-time
Open

Refactor persistent globals into RuntimeStack#464
bertysentry wants to merge 1 commit intomainfrom
462-support-pm-gawk-style-persistent-global-variables-across-scripts-by-remapping-global-slots-at-execution-time

Conversation

@bertysentry
Copy link
Copy Markdown
Contributor

Summary

  • Move persistent global layout and name tracking into RuntimeStack
  • Keep execute(...) unchanged while executePersistingGlobals(...) now merges and rebinds globals through the runtime stack
  • Update docs and tests for cross-run persistence, cleanup, and import-from-previous-execute behavior

Testing

  • Added and updated unit tests for sequential persistent runs, imported globals from a prior plain run, built-in exclusion, arrays, locals, and termination persistence
  • Ran mvn formatter:format, mvn clean test, mvn verify, and mvn verify site successfully

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d81a980fac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2339 to +2344
if (mergedGlobalLayoutActive) {
if (!hasCompatiblePersistentGlobalLayout(position.intArg(0))) {
throw new IllegalStateException(
"AVM globals are already initialized for an incompatible persistent layout.");
}
} else if (globals == null) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply per-run global seeds in persistent mode

When executePersistingGlobals(...) runs, hitting the mergedGlobalLayoutActive branch skips the executionInitialVariables application loop entirely, so non-persistent stack-managed globals from variableOverrides/runtimeArguments (notably IGNORECASE, RSTART, RLENGTH) are silently ignored for that execution. This regresses behavior compared with execute(...), where those per-run assignments are applied at SET_NUM_GLOBALS; for example, a persistent run with IGNORECASE=1 will still behave case-sensitively unless the program assigns it itself.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors how AWK global variables are tracked and persisted across repeated runs on the same AVM, moving global layout/name metadata into RuntimeStack and adding a persistent-execution API for embedders.

Changes:

  • Introduces AVM.executePersistingGlobals(...) and AVM.clearPersistentGlobals() to support cross-run persistence of user-defined globals.
  • Extends RuntimeStack to track global slot name/offset metadata and to snapshot/rebind globals by name.
  • Adds/updates docs and unit tests covering persistent globals behavior (including cross-run import, arrays, and clearing).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/main/java/io/jawk/backend/AVM.java Adds persistent execution path that merges prior user globals into the next program’s compiled global layout.
src/main/java/io/jawk/backend/RuntimeStack.java Stores global layout metadata and provides snapshot/rebind helpers used by persistent execution.
src/test/java/io/jawk/AwkTest.java Adds tests validating persistent globals behavior across sequential runs and failure modes.
src/site/markdown/java-advanced.md Documents how to use executePersistingGlobals(...) when embedding Jawk.
README.md Adds a brief note pointing embedders to executePersistingGlobals(...).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +624 to +639
Map<String, Object> carriedGlobals = collectPersistentGlobalValues();
resetTransientRuntimeState(runtimeArguments, variableOverrides);
installProgramMetadata(compiledProgram);

Map<String, Object> basePersistentSeeds = collectBasePersistentGlobalSeeds();
Map<String, Object> executionUserSeeds = collectExecutionUserGlobalSeeds(runtimeArguments, variableOverrides);
List<String> mergedGlobalNamesByOffset = buildMergedGlobalNamesByOffset(
carriedGlobals,
basePersistentSeeds,
executionUserSeeds);

runtimeStack.rebindGlobals(mergedGlobalNamesByOffset);
restoreNamedGlobals(carriedGlobals);
seedMissingNamedGlobals(carriedGlobals, basePersistentSeeds);
applyNamedGlobalOverrides(executionUserSeeds);
mergedGlobalLayoutActive = true;
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.

Support pm-gawk-style persistent global variables across scripts by remapping global slots at execution time

2 participants