lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.51k stars 164 forks source link

Representing target less expressions in ASR #1231

Open certik opened 2 years ago

certik commented 2 years ago

The following code:

from ltypes import i32

def g() -> i32:
    print("Something")
    return 3

def f() -> i32:
    2 + g()
    return 42

f()

Gives:

$ lpython --show-asr b.py
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  File "/Users/ondrej/repos/lpython/src/bin/lpython.cpp", line 1331
    return emit_asr(arg_file, lpython_pass_manager, runtime_library_dir,
  File "/Users/ondrej/repos/lpython/src/bin/lpython.cpp", line 166
    r = LFortran::LPython::python_ast_to_asr(al, *ast, diagnostics, true,
  File "/Users/ondrej/repos/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 5663
    LFORTRAN_ASSERT(asr_verify(*tu));
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 681
    v.visit_TranslationUnit(unit);
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 116
    for (auto &a : x.m_global_scope->get_scope()) {
  File "../libasr/asr.h", line 4139
  File "../libasr/asr.h", line 3900
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 299
    require(std::find(function_dependencies.begin(), function_dependencies.end(), found_dep) != function_dependencies.end(),
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 55
    throw LCompilersException("ASR verify failed: " + error_msg);
LCompilersException: ASR verify failed: Function f doesn't depend on g but is found in its dependency list.
czgdp1807 commented 2 years ago

I think its a problem with ASR verify because if you read the error message then g is actually present in f's dependency list but verify isn't able to figure that out. Let me see what I can do to fix it. Self-assigning.

czgdp1807 commented 2 years ago

Well its not a bug with dependency tracker. If I disable that check I get the following ASR for f,

f:
                (Function 
                    (SymbolTable
                        3
                        {
                            _lpython_return_variable:
                                (Variable 
                                    3 
                                    _lpython_return_variable 
                                    ReturnVar 
                                    () 
                                    () 
                                    Default 
                                    (Integer 4 []) 
                                    Source 
                                    Public 
                                    Required 
                                    .false.
                                )

                        }) 
                    f 
                    [] 
                    [] 
                    [(= 
                        (Var 3 _lpython_return_variable) 
                        (IntegerConstant 42 (Integer 4 [])) 
                        ()
                    )
                    (Return)] 
                    (Var 3 _lpython_return_variable) 
                    Source 
                    Public 
                    Implementation 
                    () 
                    .false. 
                    .false. 
                    .false. 
                    .false. 
                    .false. 
                    [] 
                    [] 
                    .false.
                )

As you can see 2 + g() statement is absent in the final ASR. So probably the bug is in AST to ASR transition.

czgdp1807 commented 2 years ago

So the question is how to deal with statements (or should we call target-less expressions?) of the kind 2 + g(). I think they should be present in the ASR because g() might be modifying a non-constant global variable so not executing 2 + g() is a mistake I think.

My conclusion - This is not a bug with dependency tracker because ASR itself is incomplete so verify is bound to fail anyways.

certik commented 2 years ago

Yes, the 2+g() must be represented in ASR. I think the immediate solution is https://github.com/lcompilers/lpython/issues/1232#issuecomment-1293624014, that is, adding an Expr stmt node.