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

Fix bug in printing a list #2654

Merged
merged 6 commits into from
Apr 19, 2024
Merged

Conversation

advikkabra
Copy link
Collaborator

Fixes #2639 .
I am not sure how/where to test this, but this fixes the issue by making an auxiliary variable in the ASR.

@Shaikh-Ubaid
Copy link
Collaborator

We should add an integration test. We can have an assert on the length of the list, couple initial elements and couple ending elements.

@Shaikh-Ubaid
Copy link
Collaborator

but this fixes the issue by making an auxiliary variable in the ASR.

Could you describe what the issue was and how creating an auxiliary variable fixes it?

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft April 16, 2024 19:53
@advikkabra
Copy link
Collaborator Author

but this fixes the issue by making an auxiliary variable in the ASR.

Could you describe what the issue was and how creating an auxiliary variable fixes it?

While calling a function which modifies the list in a print function, it repeatedly called the function. This was because in the loop made to print the list, the function was called everytime the length of the list was accessed, so it kept adding elements. By making an auxiliary variable, it keeps the length constant.

@advikkabra advikkabra marked this pull request as ready for review April 18, 2024 13:27
@Shaikh-Ubaid
Copy link
Collaborator

I checked the solution locally. It seems to fail for me.

% cat examples/expr2.py 
from lpython import i32

def test_issue_2639():
    l: list[i32] = [1, 2]

    def add_item(i: i32) -> list[i32]:
        l.append(i)
        return l

    print(add_item(3))
    print(l)
    assert len(l) == 3
    assert l[0] == 1
    assert l[1] == 2
    assert l[2] == 3


test_issue_2639()
% lpython examples/expr2.py
[1, 2, 3]
[1, 2, 3, 3, 3, 3]
AssertionError

@Shaikh-Ubaid
Copy link
Collaborator

The reason it does not fail at the CI is because in the test added we need to make a call to test_issue_2639().

@advikkabra
Copy link
Collaborator Author

advikkabra commented Apr 18, 2024

It should be fixed now. I had not been using the auxiliary variable in one place. I'm not sure why the tests are failing, because the tests pass on my machine.

@Shaikh-Ubaid
Copy link
Collaborator

The tests fail for the C backend (at the CI and on my system locally).

% ./integration_tests/run_tests.py -b c
...
99% tests passed, 1 tests failed out of 290

Label Time Summary:
c           =   2.46 sec*proc (290 tests)
cpython     =   1.78 sec*proc (275 tests)
llvm        =   2.39 sec*proc (288 tests)
wasm        =   0.83 sec*proc (38 tests)
wasm_x64    =   0.08 sec*proc (23 tests)
wasm_x86    =   0.04 sec*proc (12 tests)
x86         =   0.00 sec*proc (1 test)

Total Test time (real) =   0.77 sec

The following tests FAILED:
        109 - test_list_11 (Failed)
Errors while running CTest
Command failed: ctest -j8 --output-on-failure

@advikkabra Can you look into it?

@advikkabra
Copy link
Collaborator Author

I think the issue is with nested functions. When the function is not nested it works. I think nested functions might need to be reworked.

@Shaikh-Ubaid Shaikh-Ubaid enabled auto-merge (squash) April 19, 2024 05:26
@Shaikh-Ubaid Shaikh-Ubaid merged commit fce0b35 into lcompilers:main Apr 19, 2024
14 checks passed
assem2002 pushed a commit to assem2002/lpython that referenced this pull request Apr 28, 2024
* Fix bug in printing a list

* Update tests

* Add integration test

* Fix bugs

* Update references

* Fix tests for C backend
assem2002 pushed a commit to assem2002/lpython that referenced this pull request Apr 28, 2024
* Fix bug in printing a list

* Update tests

* Add integration test

* Fix bugs

* Update references

* Fix tests for C backend
@advikkabra advikkabra deleted the print-loop branch July 21, 2024 13: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.

Severe Bug: Calling a function inside print() leads to infinite function calls
2 participants