fix: function param shadowing and nested inline call arg resolution#46
Merged
someone235 merged 3 commits intokaspanet:masterfrom Mar 5, 2026
Merged
Conversation
Collaborator
Manyfestation
commented
Mar 5, 2026
Manyfestation
commented
- Remove constructor/constant names from env when they collide with function param names (prioritizing function parameters).
- Copy caller's _arg bindings and params into inline call env, allowing nested synthetic argument chains.
- pub const SYNTHETIC_ARG_PREFIX.
- Add regression tests for both fixes.
- Remove constructor/constant names from env when they collide with function param names (prioritizing function parameters). - Propagate caller's __arg_ bindings and params map into inline calls, allowing nested synthetic argument chains to resolve correctly. - Extract magic string into pub const SYNTHETIC_ARG_PREFIX. - Add regression tests for both fixes.
IzioDev
reviewed
Mar 5, 2026
silverscript-lang/src/compiler.rs
Outdated
| use crate::span; | ||
|
|
||
| /// Prefix used for synthetic argument bindings during inline function expansion. | ||
| pub const SYNTHETIC_ARG_PREFIX: &str = "__arg_"; |
Collaborator
There was a problem hiding this comment.
I think prefix shouldn't contains _ tail
Collaborator
Author
There was a problem hiding this comment.
The prefix itself did not change,
Just moved into a const
Collaborator
There was a problem hiding this comment.
Yes, I saw, but I don't see why we shouldn't only consider __args instead of __args_ in synthetic checks.
Unless we envision to use qualification such as __args_${scope} in the future?
Comment on lines
+3565
to
+3569
| // When a constructor constant and a function parameter share the same name, | ||
| // the function parameter value must be used (not the constant). | ||
| let source = r#" | ||
| contract Shadow(int fee) { | ||
| entrypoint function main(int fee) { |
Collaborator
There was a problem hiding this comment.
Shouldn't this even be disallowed to reduce confusion?
Scope precedence is unclear
Bonus: how should contract-level state be treated as-well
Contributor
There was a problem hiding this comment.
This is very common in most modern programming languages.
someone235
requested changes
Mar 5, 2026
Comment on lines
+3565
to
+3569
| // When a constructor constant and a function parameter share the same name, | ||
| // the function parameter value must be used (not the constant). | ||
| let source = r#" | ||
| contract Shadow(int fee) { | ||
| entrypoint function main(int fee) { |
Contributor
There was a problem hiding this comment.
This is very common in most modern programming languages.
| // inner calls can resolve arguments that flow through outer calls. | ||
| let source = r#" | ||
| contract NestedArgs() { | ||
| function inner(int x) { |
Contributor
There was a problem hiding this comment.
Can you add another level? To see the fix didn't push the bug one level further
silverscript-lang/src/compiler.rs
Outdated
| use crate::span; | ||
|
|
||
| /// Prefix used for synthetic argument bindings during inline function expansion. | ||
| pub const SYNTHETIC_ARG_PREFIX: &str = "__arg_"; |
silverscript-lang/src/compiler.rs
Outdated
|
|
||
| let mut yields: Vec<Expr<'i>> = Vec::new(); | ||
| let params = HashMap::new(); | ||
| let params = caller_params.clone(); |
Contributor
There was a problem hiding this comment.
Just rename caller_params -> params and remove this line
Merged
Fix SYNTHETIC_ARG_PREFIX
someone235
approved these changes
Mar 5, 2026
Manyfestation
added a commit
to Manyfestation/silverscript
that referenced
this pull request
Mar 5, 2026
someone235
pushed a commit
that referenced
this pull request
Mar 5, 2026
* Add debugger infrastructure + CLI debugger * fmt * Fix clippy warning and apply debugger lint fixes * Restore state transition support on debugger branch * Fix AST JSON fixtures for current type_name schema * Fix length stack cleanup and remove unused target deps * Restore typed return TypeRef model * Minimize debugger branch against upstream master * Refactor sil-debug CLI to clap and use faster-hex * Fix inline call param resolution and improve debug event tracking * Finalize debugger inline resolution and cleanup * Fix param shadowing over constructor constants * Refine debugger recorder flow and simplify compiler call sites * Fix debugger stack bindings and opcode cursor sync * Clarify debugger sequence and stepping documentation * Simplify inline resolution and debugger variable plumbing * CompileCtx * Restore pre-refactor staged debugger snapshot * Minimize compiler footprint by dropping CompileCtx refactor * Deduplicate expression recursion in resolve and inline arg expansion * Simplify inline call flow and remove synthetic arg helpers * Fix inline call bug * Fix clippy redundant pattern matching in debug session test * Rename changes * feat(debugger): port debugger onto izio spanned-ast core * fmt * debugger: fix tx-context local eval and loop index visibility * refactor: split debugger into separate crates - Rename debug.rs → debug_info.rs in silverscript-lang (types-only module) - Create debugger-session library crate (session runtime, presentation) - Create cli-debugger binary crate (renamed from sil-debug) - Migrate debug session tests and CLI smoke test to new crates - Update all imports across workspace * enhance expression resolution * Final touches , readme, tests * fmt * Fix compiler compatibility after merging origin/master * fix: align with upstream PR #44 — restore serialize_i64 encoding Bad merge resolution had reverted PR #44 changes: - Restore serialize_i64 instead of to_le_bytes+OpBin2Num for int fields - Restore Ok(9) field chunk size (was incorrectly Ok(10)) - Remove spurious OpBin2Num injection in validate_output_state - Update test expectations to match corrected encoding Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * PR cleanup - remove redundent logic & structs * Simplify fn recorder * More docs, and better namings * fix: function param shadowing and nested inline call arg resolution - Remove constructor/constant names from env when they collide with function param names (prioritizing function parameters). - Propagate caller's __arg_ bindings and params map into inline calls, allowing nested synthetic argument chains to resolve correctly. - Extract magic string into pub const SYNTHETIC_ARG_PREFIX. - Add regression tests for both fixes. * Refactor debug recorder to unify entrypoint and contract recorders * Enhance session module with failure reporting and call stack management * sync: align debugger with merged #46 and resolve merge artifacts * rename caller_params to params --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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
3 participants
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.