Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation against function call recursion #1019 #1296

Conversation

hpopov
Copy link
Contributor

@hpopov hpopov commented Nov 20, 2023

Resolves #1019.
Recursion validation implemented using BFS algorithm for nested function calls as graph nodes.

Implemented scenarios:

  1. Function calls itself def fun1(x): fun1(x);
  2. Several functions calls each other in a cycle, e.g.:
def fun1(x):
    fun2(x) + fun2(x) + fun3(x);

def fun2(x):
    fun3(x) + fun4(x);

def fun3(x):
    3*x;

def fun4(x):
    fun3(x) + fun1(x);

Here every affected call in the cycle gets a validation error (fun1::fun2, fun2::fun4, and fun4::fun1). In case a function B is called several times from a function A, only first call instance is marked with error (e.g., only first fun1::fun2).

  1. If a Module has multiple recursion cycles, all of them are identified. See example scenarios in example/recursions.calc.

Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I have only a few minor remarks.

Along the way:
- Remove redundant if-clause in evalExpr()
- Ensure only definitions are picked in checkUniqueDefinitions() (relying on name property creates a potential bug for future arithmetics grammar extensions)
- Remove else clause in bfsStep(): if parent has not been traversed before, it should be traversed even if it has a cyclic call to itself
Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

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

👍

@spoenemann spoenemann merged commit 71127c2 into eclipse-langium:main Nov 22, 2023
3 checks passed
@spoenemann spoenemann added this to the v3.0.0 milestone Jun 17, 2024
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.

Recursion should not be allowed in arithmetics
2 participants