lcompilers / lpython

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

Implement `input(prompt)` #2641

Closed kmr-srbh closed 2 months ago

kmr-srbh commented 3 months ago

Working

Without argument

name: str = input()
print("Hello,", name)
(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
LPython
Hello, LPython

With argument

from lpython import i32

def add(num1: i32, num2: i32) -> i32:
    return num1 + num2

num1: i32 = i32(int(input("Enter number 1: ")))
num2: i32 = i32(int(input("Enter number 2: ")))

print("\nTotal:", add(num1, num2))
(lp) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
Enter number 1: 2
Enter number 2: 3

Total: 5

Tasks

kmr-srbh commented 2 months ago

I am sorry for the confusion @Shaikh-Ubaid, but this is not the ASR level implementation I was talking about. I had closed the previous PR due to messy conflicts and opened a new one with the changes suggested in this thread.

Shaikh-Ubaid commented 2 months ago

@kmr-srbh Thanks for this. I think we will be needing https://github.com/lcompilers/lpython/pull/2641#discussion_r1556197690 and https://github.com/lcompilers/lpython/pull/2641#discussion_r1556198437 to proceed forward in this PR. Do you have any doubts here?

kmr-srbh commented 2 months ago

@kmr-srbh Thanks for this. I think we will be needing #2641 (comment) and #2641 (comment) to proceed forward in this PR. Do you have any doubts here?

Yes @Shaikh-Ubaid , I am confused, please guide me.

I am implementing the function as an intrinsic function in the same manner as I did in my previous PR, only the create_Input() function was added to the asr_utils.cpp file. Could you please guide me how this is getting implemented at the ASR level? Is it because of the FileRead node? Please help me.

Shaikh-Ubaid commented 2 months ago

I am implementing the function as an intrinsic function in the same manner as I did in my previous PR

What Ondrej meant by moving to some other place is that, he suggests to not implement the input as an intrinsic registry function. But instead we should simply implement it at the ASR Level directly. For an example, you can look at the commit https://github.com/lfortran/lfortran/commit/74ca3b77a00996b1b98dfa75bf85f5c1bca4ee97. In this commit we implemented create_Complex() (in your case you have create_Input()) at the ASR level directly.

kmr-srbh commented 2 months ago

What Ondrej meant by moving to some other place is that, he suggests to not implement the input as an intrinsic registry function. But instead we should simply implement it at the ASR Level directly.

Got it @Shaikh-Ubaid ! Thank you very much for the clarification! :)

I am opening a new PR with changes. I have to handle the input call in python_ast_to_asr.cpp in the way print() is handled. Please correct me if I am wrong.

Shaikh-Ubaid commented 2 months ago

I have to handle the input call in python_ast_to_asr.cpp in the way print() is handled.

Yes. You can simply have a call to your create_Input() inside if (call_name == "input").

Shaikh-Ubaid commented 2 months ago

I am opening a new PR with changes.

By the way, you can also force push your changes to your existing input branch in your fork on GitHub. After you force push, this PR will automatically get updated. Ensure that you have a copy of your original input branch so that you can restore/reference your old changes in case things go wrong.

kmr-srbh commented 2 months ago

@Shaikh-Ubaid , I am utterly confused with this :'(

I am unable to understand that if I remove the implementation of input from the intrinsic function registry, how will the create_Input() which we add to python_ast_to_asr.cpp work? How will we be creating a call to FileRead? Will we require an ASR node for Input and handle that in asr_to_llvm.cpp?

I request you to please explain me the things afresh. I am sorry for the trouble, I tried a lot :(

Shaikh-Ubaid commented 2 months ago

Just see how print is handled. Similarly handle input.

You can simply have a call to your create_Input() inside if (call_name == "input").

You can do the above. Next, inside your create_Input(), I think you will have to update the following in your original code:

        Vec<ASR::expr_t*> read_values;
        read_values.reserve(al, 1);
        read_values.push_back(al, result);

        const std::string EMPTY_STRING = "";
        body.push_back(al, ASRUtils::STMT(ASR::make_Print_t(
            al, loc, args.p, args.size(), nullptr, StringConstant(EMPTY_STRING, character(EMPTY_STRING.length())))));
        body.push_back(al, ASRUtils::STMT(ASR::make_FileRead_t(
            al, loc, 0, nullptr, nullptr, nullptr, nullptr, nullptr, read_values.p, read_values.n)));
kmr-srbh commented 2 months ago

Thanks a lot for the guidance @Shaikh-Ubaid!