Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[asr->python] initial implementation #2362

Merged
merged 18 commits into from
Mar 7, 2024

Conversation

khushi-411
Copy link
Contributor

@khushi-411 khushi-411 commented Oct 5, 2023

No description provided.

@khushi-411 khushi-411 changed the title [asr->lpython] initial implementation [asr->python] initial implementation Dec 20, 2023
@certik
Copy link
Contributor

certik commented Jan 20, 2024

We need to add tests for this, we can consider two kinds:

  • testing via our reference tests
  • testing via CPython

Both would be useful eventually to have. We can start with the first one, and later do the second one.

@certik
Copy link
Contributor

certik commented Jan 25, 2024

Try this diff:

--- a/src/libasr/codegen/asr_to_python.cpp
+++ b/src/libasr/codegen/asr_to_python.cpp
@@ -252,6 +252,10 @@ public:
         s = r;
     }
 
+    void visit_ExternalSymbol(const ASR::ExternalSymbol_t &x) {
+        visit_symbol(*x.m_external);
+    }
+
     void visit_Print(const ASR::Print_t &x) {
         std::string r;
         r += "print(";

Then a simple program works, the expr2 now gives:

$ lpython --show-python examples/expr2.py
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  File "/Users/ondrej/repos/lpython/src/bin/lpython.cpp", line 1845
    return emit_python(arg_file, runtime_library_dir, compiler_options);
  File "/Users/ondrej/repos/lpython/src/bin/lpython.cpp", line 429
    LCompilers::Result<std::string> res = LCompilers::asr_to_lpython(al, *asr, diagnostics, compiler_options, color, indent);
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_python.cpp", line 338
    v.visit_TranslationUnit(asr);
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_python.cpp", line 176
    visit_symbol(*item.second);
  File "../libasr/asr.h", line 5060
  File "../libasr/asr.h", line 4774
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_python.cpp", line 217
    visit_symbol(*item.second);
  File "../libasr/asr.h", line 5060
  File "../libasr/asr.h", line 4778
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_python.cpp", line 256
    visit_symbol(*x.m_external);
  File "../libasr/asr.h", line 5060
  File "../libasr/asr.h", line 4775
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_python.cpp", line 205
    visit_body(x, r, true);
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_python.cpp", line 120
    visit_stmt(*x.m_body[i]);
  File "../libasr/asr.h", line 5077
  File "../libasr/asr.h", line 4811
  File "../libasr/asr.h", line 5093
LCompilersException: visit_If() not implemented

So just visit_If has to be implemented as well.

@khushi-411
Copy link
Contributor Author

khushi-411 commented Mar 5, 2024

Hi, @certik! The PR is ready for your review :-)

Here's the output of the generated code:

def test_boolOp():
    a: bool
    b: bool
    a = False
    b = True
    a = a and b
    b = a or True
    a = a or b
    a = a and (b == b)
    a = a and (b != b)
    a = b or b

And here's the original code:

def test_boolOp():
    a: bool
    b: bool
    a = False
    b = True
    a = a and b
    b = a or True
    a = a or b
    a = a and b == b
    a = a and b != b
    a = b or b

I confirmed the generated and original codes are the same for other tests, too.

Thanks!

@khushi-411 khushi-411 marked this pull request as ready for review March 5, 2024 10:56
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
@khushi-411 khushi-411 marked this pull request as draft March 5, 2024 12:51
@khushi-411 khushi-411 marked this pull request as ready for review March 5, 2024 18:44
@khushi-411
Copy link
Contributor Author

@certik, @Thirumalai-Shaktivel, this PR is ready to be merged. Gentle ping. Thanks!

r.append(x.m_name);
r += "(";
for (size_t i = 0; i < x.n_args; i++) {
visit_expr(*x.m_args[i]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to specify the datatype of the argument here, right?

Something like:

def f(x: i32):
	pass

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I'll work on this in a follow-up PR. I added a TODO remark so that we don't miss it in future.

Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, Looks great!

src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
src/libasr/codegen/asr_to_python.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,9 @@
def if_check():
Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It went unnoticed, expr17.py is not registered in tests.
You can submit a new PR for now.

@Thirumalai-Shaktivel Thirumalai-Shaktivel merged commit 0bcc640 into lcompilers:main Mar 7, 2024
13 checks passed
@khushi-411
Copy link
Contributor Author

Yay! Thanks for all your informative reviews, @certik & @Thirumalai-Shaktivel!

@khushi-411 khushi-411 deleted the backend_lpython branch March 7, 2024 06:37
hankluo6 pushed a commit to hankluo6/lpython that referenced this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants