| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@Shaikh-Ubaid I am repurposing the Reverse() utility function for lists to implement this function. Please tell me if it is the correct way to do this. I will then implement a similar utility function for tuples also. Another way I think is to handle both the cases in one single function. We can call the reverse utility function for lists from within. We will still need one more function for tuples though. I request you to guide me on this. |
Sorry, something went wrong.
|
@Shaikh-Ubaid , the tests for cpython fail as the function returns an iterator there. We could do list(reversed(...)), but this does not work in LPython. It treats the function call as it is and throws an error on llvm code generation. |
Sorry, something went wrong.
|
|
||
| enum class IntrinsicElementalFunctions : int64_t { | ||
| ObjectType, | ||
| Reversed, |
There was a problem hiding this comment.
Reversed returns an iterator, but does not actually reverse a list. I think we do not have support for iterators yet in LPython.
The approach in this PR reverses the entire list, which I think is not the correct approach.
Sorry, something went wrong.
There was a problem hiding this comment.
@Shaikh-Ubaid I return a reversed list using the approach used for dict.keys (#1881 (comment)) as we do not support iterators now.
Sorry, something went wrong.
|
The current approach in this PR reverses the original list which we should not be doing. As a result, it fails for the following: % git diff
diff --git a/integration_tests/test_builtin_reversed.py b/integration_tests/test_builtin_reversed.py
index 51cf3607f..2f38e2e09 100644
--- a/integration_tests/test_builtin_reversed.py
+++ b/integration_tests/test_builtin_reversed.py
@@ -15,6 +15,7 @@ def test_builtin_reversed():
reversed_numbers: list[i32] = reversed(numbers)
print(reversed_numbers)
assert reversed_numbers == [5, 4, 3, 2, 1]
+ assert numbers == [1, 2, 3, 4, 5]
# list returned through function call
alphabet_dictionary: dict[str, i32] = {% lpython integration_tests/test_builtin_reversed.py
['e', 'd', 'c', 'b', 'a']
[5, 4, 3, 2, 1]
AssertionError |
Sorry, something went wrong.
Thank you very much for catching this @Shaikh-Ubaid ! I totally missed that I was modifying the original list. I think we can create a new function in asr_to_llvm.cpp and call listreverse() from there for lists. We can create a new list copy and do all the work. Shall we move ahead with the idea? Tuples can be handled similarly. |
Sorry, something went wrong.
I think we should not do that. Iterators do not make a copy. They simply work like a pointer which can be used to traverse an object. reversed will return an iterator that will be used to traverse the object in the opposite direction from the other end. I would not implement reversed at the moment until we have support for iterator. I am not yet sure if we need support for iterator at the ASR level. I asked about it here https://lfortran.zulipchat.com/#narrow/stream/311866-LPython/topic/Iterator/near/432632427. |
Sorry, something went wrong.
|
I think it is correct to either implement support for an iterator or wait for it. 👍 |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Working