Skip to content

feat(expansion): implement nameref resolution for variable access#1039

Open
lu-zero wants to merge 1 commit into
reubeno:mainfrom
lu-zero:nameref
Open

feat(expansion): implement nameref resolution for variable access#1039
lu-zero wants to merge 1 commit into
reubeno:mainfrom
lu-zero:nameref

Conversation

@lu-zero

@lu-zero lu-zero commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

When a variable is declared as a nameref (declare -n ref=target), accessing or assigning to $ref now correctly redirects to $target.

  • Add resolve_nameref_target() helper to resolve nameref chains
  • Resolve namerefs in expand_parameter for all variable access patterns:
    • Simple variable expansion ($ref)
    • Indexed array access (${ref[index]})
    • All indices (${!ref[@]})
    • All values (${ref[@]})
  • Resolve namerefs in assign_to_parameter for write operations
  • Resolve namerefs in apply_assignment for direct assignments

This enables proper support for nameref variables used in functions and scripts that pass array references.

Adds comprehensive YAML tests for nameref functionality including:

  • Basic read/write through reference
  • Array and associative array access
  • Pass-by-reference function arguments
  • Array iteration helpers

Updates existing helper functions to use nameref properly now that it is implemented.

@github-actions

github-actions Bot commented Mar 3, 2026

Copy link
Copy Markdown

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 17.30 μs 17.34 μs 0.03 μs ⚪ Unchanged
eval_arithmetic 0.15 μs 0.15 μs 0.00 μs ⚪ Unchanged
expand_one_string 1.54 μs 1.55 μs 0.01 μs ⚪ Unchanged
for_loop 25.64 μs 25.49 μs -0.15 μs ⚪ Unchanged
full_peg_complex 58.20 μs 57.86 μs -0.34 μs ⚪ Unchanged
full_peg_for_loop 6.24 μs 6.17 μs -0.07 μs ⚪ Unchanged
full_peg_nested_expansions 16.70 μs 16.73 μs 0.02 μs ⚪ Unchanged
full_peg_pipeline 4.24 μs 4.33 μs 0.09 μs 🟠 +2.19%
full_peg_simple 1.79 μs 1.86 μs 0.07 μs 🟠 +3.86%
function_call 2.75 μs 2.87 μs 0.12 μs ⚪ Unchanged
instantiate_shell 55.23 μs 54.41 μs -0.83 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 26302.99 μs 26323.92 μs 20.93 μs ⚪ Unchanged
parse_peg_bash_completion 2067.56 μs 2063.93 μs -3.62 μs ⚪ Unchanged
parse_peg_complex 19.94 μs 19.84 μs -0.10 μs ⚪ Unchanged
parse_peg_for_loop 1.98 μs 1.96 μs -0.02 μs 🟢 -1.06%
parse_peg_pipeline 2.16 μs 2.05 μs -0.11 μs 🟢 -5.00%
parse_peg_simple 1.08 μs 1.11 μs 0.04 μs 🟠 +3.24%
run_echo_builtin_command 17.96 μs 18.50 μs 0.54 μs 🟠 +3.00%
tokenize_sample_script 3.45 μs 3.52 μs 0.07 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-core/src/expansion.rs 🟢 95.45% 🟢 95.53% 🟢 0.08%
Overall Coverage 🟢 75.01% 🟢 75.04% 🟢 0.03%

Minimum allowed coverage is 70%, this run produced 75.04%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1578 74.82
❗️ Error 19 0.90
❌ Fail 158 7.49
⏩ Skip 339 16.07
❎ Expected Fail 13 0.62
✔️ Unexpected Pass 2 0.09
📊 Total 2109 100.00

When a variable is declared as a nameref (declare -n ref=target),
accessing or assigning to $ref now correctly redirects to $target.

- Add resolve_nameref_target() helper to resolve nameref chains
- Resolve namerefs in expand_parameter for all variable access patterns:
  - Simple variable expansion ($ref)
  - Indexed array access (${ref[index]})
  - All indices (${!ref[@]})
  - All values (${ref[@]})
- Resolve namerefs in assign_to_parameter for write operations
- Resolve namerefs in apply_assignment for direct assignments

This enables proper support for nameref variables used in functions
and scripts that pass array references.

Adds comprehensive YAML tests for nameref functionality including:
- Basic read/write through reference
- Array and associative array access
- Pass-by-reference function arguments
- Array iteration helpers

Updates existing helper functions to use nameref properly now that
it is implemented.
@reubeno reubeno added state: needs review PR or issue author is waiting for a review area: compat Related to compatibility with standard shells labels Mar 14, 2026
@reubeno

reubeno commented Mar 16, 2026

Copy link
Copy Markdown
Owner

Thanks for your patience with this one, @lu-zero.

I looked into it a bit -- this gets us some partial progress, but I think the nameref implementation needs to be plumbed through more deeply (and widely). There's other code paths that can assign to or query variables (e.g., builtins that update variables) and which also need to be enlightened to honor namerefs.

My perspective: env.rs / variables.rs will need to know about namerefs, or at least some fundamental aliasing. We'll also need to validate whether a nameref chain (a -> b -> c) needs to be handled, how to deal with cycles, etc.

If I have some time, I'm game to take a crack at the above -- using your changes as a starting point. That may not be until a future weekend, though.

Was this resolving something that was blocking something you were working on?

@reubeno reubeno removed the state: needs review PR or issue author is waiting for a review label Mar 16, 2026
@lu-zero

lu-zero commented Mar 16, 2026

Copy link
Copy Markdown
Contributor Author

it is one of the changes needed to support Gentoo portage parsing so yes ^^;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: compat Related to compatibility with standard shells

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants