Interactive Shell: Printing top level expression #2698

Vipul-Cariappa · 2024-05-12T14:29:24Z

Example:

Present behavior:

>>> 1 + 2
>>>

Expected behavior (after potentially merging this PR):

>>> 1 + 2
3
>>>

Reference for myself
Things to do:

Vipul-Cariappa · 2024-05-12T14:30:48Z

src/libasr/pass/global_stmts.cpp

@@ -91,6 +91,7 @@ void pass_wrap_global_stmts(Allocator &al,
                target = return_var_ref;
                idx++;
            } else {
+                // TODO: We will need to add support to return other ASR::ttypeType::*


We will need to handle all other cases like list, set, dict, etc.

Tests are going to fail due to this reason.

Vipul-Cariappa · 2024-05-12T14:38:08Z

src/libasr/pass/global_stmts.cpp

@@ -119,7 +120,7 @@ void pass_wrap_global_stmts(Allocator &al,
        /* a_body */ body.p,
        /* n_body */ body.size(),
        /* a_return_var */ (return_var ? return_var_ref : nullptr),
-        (return_var ? ASR::abiType::BindC : ASR::abiType::Source),
+        ASR::abiType::Source,


I have changed this because asr_to_llvm changes the final function depending on ABI type (Look at

lpython/src/libasr/codegen/asr_to_llvm.cpp

Lines 3794 to 3805 in 476d9bb

if (ASRUtils::get_FunctionType(x)->m_abi == ASR::abiType::BindC) {

if (ASRUtils::get_FunctionType(x)->m_bindc_name) {

fn_name = ASRUtils::get_FunctionType(x)->m_bindc_name;

} else {

fn_name = sym_name;

}

} else if (ASRUtils::get_FunctionType(x)->m_deftype == ASR::deftypeType::Interface &&

ASRUtils::get_FunctionType(x)->m_abi != ASR::abiType::Intrinsic) {

fn_name = sym_name;

} else {

fn_name = mangle_prefix + sym_name;

}

). We don't want the function name to change, it becomes difficult to figure out the correct function name in PythonCompile.evaluate.

Can you check if this change works with LFortran? Simply submit a PR with this exact change and we can see if the CI passes.

Vipul-Cariappa · 2024-05-12T14:43:49Z

src/lpython/python_evaluator.cpp

+        } else if (return_type == "none") {
+            result.type = EvalResult::none;
+        } else {
+            throw LCompilersException("FortranEvaluator::evaluate(): Return type not supported");
+        }


I was thinking we could accept struct as the return type here for dict, list, etc. And handle it accordingly.
But I have no idea how would we print it later.
Example:

>>> n: list[i32] = [1, 2, 3] >>> n.append(4) >>> n

We would like to print the list n in the last line. But it would be returned as a struct here.

Let's wrap this code into a new function that returns the return variable (or populates it when passed as argument).

Ok will do it in the next commit.

Vipul-Cariappa · 2024-05-12T14:44:59Z

src/lpython/semantics/python_ast_to_asr.cpp

@@ -6691,8 +6699,7 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
        // If tmp is a statement and not an expression
        // never cast into expression using ASRUtils::EXPR
        // Just ignore and exit the function naturally.
-        if( tmp && !ASR::is_a<ASR::stmt_t>(*tmp) ) {
-            LCOMPILERS_ASSERT(ASR::is_a<ASR::expr_t>(*tmp));
+        if( tmp && !ASR::is_a<ASR::expr_t>(*tmp) ) {


This is changed to keep track of the final expression executed, and printing its result.

I didn't get this. Could you elaborate this, please?

Please look at the difference in the AST generated with this change and without this change (i.e. this PR and the main branch). It may become easier to understand. Start the interactive shell with -v option to print the AST and LLVM IR.

With this change, the pass_wrap_global_stmt will generate a function that returns the result of the expression tmp, avoiding the modifications you specified with make_dummy_assignment (#2698 (review)).

Vipul-Cariappa · 2024-05-12T14:49:51Z

@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.

ubaidsk

I think LFortran already prints values for global expressions. We just need to follow how it does it.

In LFortran, the pass pass_wrap_global_stmt is able to give us a function that can return a value. The same does not hold true for LPython. I think it happens because of the make_dummy_assignment used here

lpython/src/lpython/semantics/python_ast_to_asr.cpp

Lines 4840 to 4850 in 476d9bb

    
               if (ASR::is_a<ASR::expr_t>(*tmp)) { 
        
                   tmp = make_dummy_assignment(ASRUtils::EXPR(tmp)); 
        
               } 
        
               ASR::stmt_t* tmp_stmt = ASRUtils::STMT(tmp); 
        
               body.push_back(al, tmp_stmt); 
        
           } else if (!tmp_vec.empty()) { 
        
               for (auto t: tmp_vec) { 
        
                   if (t != nullptr) { 
        
                       if (ASR::is_a<ASR::expr_t>(*t)) { 
        
                           t = make_dummy_assignment(ASRUtils::EXPR(t)); 
        
                       }

So, we just need to figure out how to avoid creating the dummy assignment (without breaking any existing tests) and we should have it.

ubaidsk · 2024-05-12T16:58:36Z

src/libasr/pass/global_stmts.cpp

@@ -119,7 +120,7 @@ void pass_wrap_global_stmts(Allocator &al,
        /* a_body */ body.p,
        /* n_body */ body.size(),
        /* a_return_var */ (return_var ? return_var_ref : nullptr),
-        (return_var ? ASR::abiType::BindC : ASR::abiType::Source),
+        ASR::abiType::Source,


Can you check if this change works with LFortran? Simply submit a PR with this exact change and we can see if the CI passes.

ubaidsk · 2024-05-12T16:59:57Z

src/lpython/python_evaluator.cpp

+        } else if (return_type == "none") {
+            result.type = EvalResult::none;
+        } else {
+            throw LCompilersException("FortranEvaluator::evaluate(): Return type not supported");
+        }


Let's wrap this code into a new function that returns the return variable (or populates it when passed as argument).

ubaidsk · 2024-05-12T17:01:54Z

src/lpython/semantics/python_ast_to_asr.cpp

+            module_sym->m_dependencies = current_module_dependencies.p;
+            module_sym->n_dependencies = current_module_dependencies.size();
+            if (!overload_defs.empty()) {
+                create_GenericProcedure(x.base.base.loc);
+            }


This portion seems to be the same as that of the if part. So, let's move this out of the if-else?

Yes, we should be able to do that. Will do it in the next commit.

ubaidsk · 2024-05-12T17:03:01Z

src/lpython/semantics/python_ast_to_asr.cpp

@@ -6691,8 +6699,7 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
        // If tmp is a statement and not an expression
        // never cast into expression using ASRUtils::EXPR
        // Just ignore and exit the function naturally.
-        if( tmp && !ASR::is_a<ASR::stmt_t>(*tmp) ) {
-            LCOMPILERS_ASSERT(ASR::is_a<ASR::expr_t>(*tmp));
+        if( tmp && !ASR::is_a<ASR::expr_t>(*tmp) ) {


I didn't get this. Could you elaborate this, please?

ubaidsk · 2024-05-12T17:12:54Z

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 · 2024-05-13T04:35:48Z

Replying to #2698 (review)

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

lpython/src/lpython/python_evaluator.cpp

Lines 114 to 149 in 6aed371

    
           e->add_module(std::move(m)); 
        
           if (call_run_fn) { 
        
               if (return_type == "integer4") { 
        
                   int32_t r = e->int32fn(run_fn); 
        
                   result.type = EvalResult::integer4; 
        
                   result.i32 = r; 
        
               } else if (return_type == "integer8") { 
        
                   int64_t r = e->int64fn(run_fn); 
        
                   result.type = EvalResult::integer8; 
        
                   result.i64 = r; 
        
               } else if (return_type == "real4") { 
        
                   float r = e->floatfn(run_fn); 
        
                   result.type = EvalResult::real4; 
        
                   result.f32 = r; 
        
               } else if (return_type == "real8") { 
        
                   double r = e->doublefn(run_fn); 
        
                   result.type = EvalResult::real8; 
        
                   result.f64 = r; 
        
               } else if (return_type == "complex4") { 
        
                   std::complex<float> r = e->complex4fn(run_fn); 
        
                   result.type = EvalResult::complex4; 
        
                   result.c32.re = r.real(); 
        
                   result.c32.im = r.imag(); 
        
               } else if (return_type == "complex8") { 
        
                   std::complex<double> r = e->complex8fn(run_fn); 
        
                   result.type = EvalResult::complex8; 
        
                   result.c64.re = r.real(); 
        
                   result.c64.im = r.imag(); 
        
               } else if (return_type == "void") { 
        
                   e->voidfn(run_fn); 
        
                   result.type = EvalResult::statement; 
        
               } else if (return_type == "none") { 
        
                   result.type = EvalResult::none; 
        
               } else { 
        
                   throw LCompilersException("FortranEvaluator::evaluate(): Return type not supported"); 
        
               }

). And then being printed by interactive_python_repl (Look at

lpython/src/bin/lpython.cpp

Lines 890 to 944 in 6aed371

    
           switch (r.type) { 
        
               case (LCompilers::PythonCompiler::EvalResult::integer4) : { 
        
                   if (verbose) std::cout << "Return type: integer" << std::endl; 
        
                   if (verbose) section("Result:"); 
        
                   std::cout << r.i32 << std::endl; 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::integer8) : { 
        
                   if (verbose) std::cout << "Return type: integer(8)" << std::endl; 
        
                   if (verbose) section("Result:"); 
        
                   std::cout << r.i64 << std::endl; 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::real4) : { 
        
                   if (verbose) std::cout << "Return type: real" << std::endl; 
        
                   if (verbose) section("Result:"); 
        
                   std::cout << std::setprecision(8) << r.f32 << std::endl; 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::real8) : { 
        
                   if (verbose) std::cout << "Return type: real(8)" << std::endl; 
        
                   if (verbose) section("Result:"); 
        
                   std::cout << std::setprecision(17) << r.f64 << std::endl; 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::complex4) : { 
        
                   if (verbose) std::cout << "Return type: complex" << std::endl; 
        
                   if (verbose) section("Result:"); 
        
                   std::cout << std::setprecision(8) << "(" << r.c32.re << ", " << r.c32.im << ")" << std::endl; 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::complex8) : { 
        
                   if (verbose) std::cout << "Return type: complex(8)" << std::endl; 
        
                   if (verbose) section("Result:"); 
        
                   std::cout << std::setprecision(17) << "(" << r.c64.re << ", " << r.c64.im << ")" << std::endl; 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::statement) : { 
        
                   if (verbose) { 
        
                       std::cout << "Return type: none" << std::endl; 
        
                       section("Result:"); 
        
                       std::cout << "(statement)" << std::endl; 
        
                   } 
        
                   break; 
        
               } 
        
               case (LCompilers::PythonCompiler::EvalResult::none) : { 
        
                   if (verbose) { 
        
                       std::cout << "Return type: none" << std::endl; 
        
                       section("Result:"); 
        
                       std::cout << "(nothing to execute)" << std::endl; 
        
                   } 
        
                   break; 
        
               } 
        
               default : throw LCompilers::LCompilersException("Return type not supported"); 
        
           }

).

ubaidsk · 2024-05-27T16:40:04Z

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

Vipul-Cariappa · 2024-05-28T00:41:08Z

Closed by #2716

work towards printing top level expression

6aed371

Vipul-Cariappa marked this pull request as draft May 12, 2024 14:29

Vipul-Cariappa commented May 12, 2024

View reviewed changes

ubaidsk reviewed May 12, 2024

View reviewed changes

Vipul-Cariappa mentioned this pull request May 13, 2024

pass_wrap_global_stmts always creates function of abiType::BindC #2700

Closed

Vipul-Cariappa mentioned this pull request May 25, 2024

Printing top-level Expressions #2716

Merged

Vipul-Cariappa closed this May 28, 2024

Vipul-Cariappa deleted the interactive branch May 28, 2024 10:35

	if (ASRUtils::get_FunctionType(x)->m_abi == ASR::abiType::BindC) {
	if (ASRUtils::get_FunctionType(x)->m_bindc_name) {
	fn_name = ASRUtils::get_FunctionType(x)->m_bindc_name;
	} else {
	fn_name = sym_name;
	}
	} else if (ASRUtils::get_FunctionType(x)->m_deftype == ASR::deftypeType::Interface &&
	ASRUtils::get_FunctionType(x)->m_abi != ASR::abiType::Intrinsic) {
	fn_name = sym_name;
	} else {
	fn_name = mangle_prefix + sym_name;
	}

	if (ASR::is_a<ASR::expr_t>(*tmp)) {
	tmp = make_dummy_assignment(ASRUtils::EXPR(tmp));
	}
	ASR::stmt_t* tmp_stmt = ASRUtils::STMT(tmp);
	body.push_back(al, tmp_stmt);
	} else if (!tmp_vec.empty()) {
	for (auto t: tmp_vec) {
	if (t != nullptr) {
	if (ASR::is_a<ASR::expr_t>(*t)) {
	t = make_dummy_assignment(ASRUtils::EXPR(t));
	}

Interactive Shell: Printing top level expression #2698

Interactive Shell: Printing top level expression #2698

Uh oh!

Conversation

Vipul-Cariappa commented May 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vipul-Cariappa commented May 12, 2024

Uh oh!

ubaidsk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ubaidsk commented May 12, 2024

Uh oh!

Vipul-Cariappa commented May 13, 2024

Uh oh!

ubaidsk commented May 27, 2024

Uh oh!

Vipul-Cariappa commented May 28, 2024

Uh oh!

Uh oh!

Vipul-Cariappa commented May 12, 2024 •

edited

Loading