Skip to content

Sync LibASR with LFortran #2836

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

Merged
merged 45 commits into from
Mar 30, 2025
Merged

Conversation

ubaidsk
Copy link
Collaborator

@ubaidsk ubaidsk commented Mar 30, 2025

PR at LFortran lfortran/lfortran#6780.

@ubaidsk ubaidsk force-pushed the ubaid/pr/libasr_sync branch 3 times, most recently from 17cc9c4 to 6d6270d Compare March 30, 2025 04:34
@ubaidsk

This comment was marked as resolved.

@ubaidsk ubaidsk force-pushed the ubaid/pr/libasr_sync branch from c95f275 to aa3b119 Compare March 30, 2025 06:13
@ubaidsk ubaidsk force-pushed the ubaid/pr/libasr_sync branch from aa3b119 to ade4859 Compare March 30, 2025 06:16
@ubaidsk ubaidsk marked this pull request as ready for review March 30, 2025 06:22
@certik
Copy link
Contributor

certik commented Mar 30, 2025

Locally I got:

[ 98%] Linking CXX executable test_stacktrace
[ 99%] Linking CXX executable test_lpython
[ 99%] Built target test_stacktrace
[ 99%] Built target test_lpython
[100%] Linking CXX executable lpython
/bin/sh: ../../lfortran/libasr/dwarf_convert.py: No such file or directory
make[2]: *** [src/bin/CMakeFiles/lpython.dir/build.make:153: src/bin/lpython] Error 127
make[2]: *** Deleting file 'src/bin/lpython'
make[1]: *** [CMakeFiles/Makefile2:457: src/bin/CMakeFiles/lpython.dir/all] Error 2
make: *** [Makefile:166: all] Error 2

@ubaidsk
Copy link
Collaborator Author

ubaidsk commented Mar 30, 2025

Locally I got:

I didn't see those locally. What build steps did you use?

@certik
Copy link
Contributor

certik commented Mar 30, 2025

I just used the usual ./build0.sh && ./build1.sh. I use macOS, so it it calling that Python script which had incorrect path. I fixed it.

@certik
Copy link
Contributor

certik commented Mar 30, 2025

Main has 363 integration tests. This PR has 204. So we have 159 tests that we need to fix. The classes of tests that we disabled are:

  • Some array tests
  • Almost all bind(c) tests (C interop), and all symbolics
  • A lot of string, list, dict, elemental and math functions,
  • A lot of structs
  • lpython decorator

So essential functionality, but it hopefully shouldn't be hard to get it working again, since the code in libasr is available at https://github.com/lcompilers/lpython/tree/635ed2020123199c22f25d3dd23056cd80717762/src/libasr and we just need to port the implementation from it to the newest LPython by:

  • port a given change to libasr/* (create a branch in the submodule there)
  • port any changes in the frontend needed
  • enable some test, ensure it works locally
  • commit the new submodule hash (that contains your branch)
  • push the libasr branch into your fork of lfortran at github
  • send a PR against LPython, which will contain the new hash for libasr, and it will be accessible because you pushed into your fork in the previous step; ensure CI passes
  • create a PR in LFortran using your pushed branch in the fork; ensure CI passes, and add any LFortran tests that are needed so that the change is fully tested in LFortran, which acts as the upstream for libasr, so that the new change doesn't get broken as LFortran is developed.
  • Merge the LFortran PR, ensure the LPython PR is pointing to a hash from the main branch of LFortran (either by updating the hash to the latest main, or by ensuring the LFortran PR merged the commit that the LPython PR depended on)
  • Merge the LPython PR

Before doing some big refactoring or feature in libasr, it's a good idea to first update the submodule to the latest main of LFortran and ensure everything passes. It's a good idea to regularly do this update, to keep in sync.

Some other things to fix:

  • in src/CMakeLists.txt remove add_subdirectory(${CMAKE_SOURCE_DIR}/libasr/src/libasr ${CMAKE_BINARY_DIR}/libasr) and rather source it from the root CMake. Also do not use ${CMAKE_BINARY_DIR}/ for add_subdirectory.

@certik certik merged commit 1e0d300 into lcompilers:main Mar 30, 2025
18 checks passed
@ubaidsk ubaidsk deleted the ubaid/pr/libasr_sync branch March 31, 2025 00:36
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