Skip to content

refactor(builtins): replace magic variable hack with BuiltinSideEffect enum#750

Merged
chaliy merged 1 commit intomainfrom
claude/issue-728-side-effects-lKkKE
Mar 18, 2026
Merged

refactor(builtins): replace magic variable hack with BuiltinSideEffect enum#750
chaliy merged 1 commit intomainfrom
claude/issue-728-side-effects-lKkKE

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented Mar 18, 2026

Summary

  • Introduce BuiltinSideEffect enum on ExecResult as a typed channel for builtins to communicate state changes to the interpreter
  • Migrate Set (_SET_POSITIONAL), Shift (_SHIFT_COUNT), and Read -a (_ARRAY_READ_*) from magic-prefixed variables to structured side effects
  • Add apply_builtin_side_effects() method to interpreter that processes the enum variants
  • Defense-in-depth: is_internal_variable() still blocks all legacy prefixes

Closes #728

Test plan

  • All 2022+ unit tests pass
  • All threat model security tests pass (TM-INJ-009 prefix injection blocked)
  • read -a array tests updated to verify side effects
  • cargo fmt --check and cargo clippy clean

…t enum

Introduce BuiltinSideEffect enum on ExecResult for builtins to communicate
state changes to the interpreter via a typed channel instead of smuggling
data through magic-prefixed variables in the HashMap.

Migrated: Set (_SET_POSITIONAL), Shift (_SHIFT_COUNT), Read -a
(_ARRAY_READ_*). Remaining magic vars (_NAMEREF_, _READONLY_, etc.) are
still in use and will be migrated in follow-up work.

Closes #728
@chaliy chaliy merged commit 4878361 into main Mar 18, 2026
23 checks passed
@chaliy chaliy deleted the claude/issue-728-side-effects-lKkKE branch March 18, 2026 03:22
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.

refactor: replace magic variable hack for builtin side-effects

1 participant