Store function call state in a stack #294
Open
Conversation
This file contains 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
As functions are called store the current execution state for the
function in a stack. This includes the local variables and re.group.{N}
variables.
This stack is used to detect recursion and prevent an interpreter panic.
In the future this information could be used for including a VCL stack
trace in error output or for accessing calling function state in the
debugger.
richardmarshall
force-pushed
the
recursive_function_calls
branch
from
b26ea74
to
c5db1f1
Compare
ysugimoto
reviewed
Apr 4, 2024
| Regex: make(map[string]*value.String), | ||
| Subroutine: sub, | ||
| } | ||
| i.Stack = append(i.Stack, sf) |
There was a problem hiding this comment.
Tentatively we can define the max call stack frames and raise an error if exceeded, probably around 100 would be enough but up to you.
There was a problem hiding this comment.
Yeah that seems reasonable. I did a test and generated a silly call chain of 200 functions in a fiddle and it didn't cause an error. Seems like the limiting factor isn't the call depth but the overall workspace memory usage which would eventually be exhausted.
2 tasks
richardmarshall
force-pushed
the
recursive_function_calls
branch
from
f07942c
to
a42813b
Compare
|
@richardmarshall Sorry I forgot to merge this PR and treid to resolve conflicts but some testing errors are found due to the head branch changes. I'm so happy if you could adjust to main branch but of course I could do after merging this PR, thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As functions are called store the current execution state for the function in a stack. This includes the local variables and re.group.{N} variables.
This stack is used to detect recursion and prevent an interpreter panic.
In the future this information could be used for including a VCL stack trace in error output or for accessing calling function state in the debugger.
Note: The current code structure makes accessing the re.group variables directly from the stack pointer a challenge. To keep things simple I retained the save / restore model we currently have for those. If there is consensus on the idea of using a stack frame model we can decide if it makes sense to restructure things to make access through the stack pointer possible.
Resolves #293, #297