search

lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.37k stars 156 forks source link

Interactive Shell: Printing top level expression #2698

Closed Vipul-Cariappa closed 1 month ago

Vipul-Cariappa commented 1 month ago

Example:

Present behavior:

>>> 1 + 2
>>>

Expected behavior (after potentially merging this PR):

>>> 1 + 2
3
>>>

Reference for myself Things to do:

Vipul-Cariappa commented 1 month ago

@Shaikh-Ubaid, Please have a look at the comments I have left. Do let me know, if I am not clear.

After having a look at the comments, read the following part:

We can otherwise implement it as an ASR pass. Identify the last expression and wrap the print function around it. This ASR pass will only be enabled when using the interactive mode.

Shaikh-Ubaid commented 1 month ago

We can otherwise implement it as an ASR pass. Identify the last expression and wrap the print function around it. This ASR pass will only be enabled when using the interactive mode.

I clarified this in my review comment above.

Also, you need to update the reference tests for the CI to pass.

Vipul-Cariappa commented 1 month ago

Replying to https://github.com/lcompilers/lpython/pull/2698#pullrequestreview-2051417737

Let's say we made the necessary modifications such that we return a value from the function generated from pass_wrap_global_stmt. But what if the return type is list or dict. We would not be able to print it. Code to print list or dict should be produced at the LLVM backend level.

Reference: The function generated by pass_wrap_global_stmt is being called by PythonCompiler::evaluate (look at https://github.com/lcompilers/lpython/blob/6aed371ba12a2b55e7284483dad651aa3982600e/src/lpython/python_evaluator.cpp#L114-L149). And then being printed by interactive_python_repl (Look at https://github.com/lcompilers/lpython/blob/6aed371ba12a2b55e7284483dad651aa3982600e/src/bin/lpython.cpp#L890-L944).

Shaikh-Ubaid commented 1 month ago

@Vipul-Cariappa Is this safe to close? If it is, can you close this?

Vipul-Cariappa commented 1 month ago

Closed by #2716