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

Support ccall() for symengine and other libs #2196

Merged
merged 4 commits into from
Jul 20, 2023

Conversation

Shaikh-Ubaid
Copy link
Collaborator

No description provided.

@Shaikh-Ubaid Shaikh-Ubaid changed the title Fix ccall for cpython Support ccall() for symengine and other libs Jul 20, 2023
@Shaikh-Ubaid
Copy link
Collaborator Author

@anutosh491 It seems to work, but there is a catch.

$ cat integration_tests/symbolics_07.py                    
from lpython import ccall, CPtr
import os

@ccall(header="symengine/cwrapper.h", py_mod="symengine", py_mod_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_new_heap() -> CPtr:
    pass

@ccall(header="symengine/cwrapper.h", py_mod="symengine", py_mod_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_const_pi(x: CPtr) -> None:
    pass

@ccall(header="symengine/cwrapper.h", py_mod="symengine", py_mod_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_str(x: CPtr) -> str:
    pass

def main0():
    x: CPtr = basic_new_heap()
    basic_const_pi(x)
    s: str = basic_str(x)
    print(s)
    # assert s == "pi"

main0()
$ python integration_tests/symbolics_07.py                    
b'pi'
$ lpython integration_tests/symbolics_07.py --enable-symengine --backend c
pi 

The outputs by lpython and cpython do not match. Therefore the assert in the integration_tests/symbolics_07.py fails (I temporarily commented it out).

@anutosh491
Copy link
Collaborator

anutosh491 commented Jul 20, 2023

I went through the example above and I think we get the b'pi' output through CPython while using ccall because ctypes library returns strings as bytes objects when calling C functions that return strings

@Shaikh-Ubaid
Copy link
Collaborator Author

I went through the example above and I think we get the b'pi' output through CPython while using ccall because ctypes library returns strings as bytes objects when calling C functions that return strings

Good catch @anutosh491! Thank you. I fixed it in lpython.py: Decode ctypes strings before returning.

@Shaikh-Ubaid
Copy link
Collaborator Author

Ready.

@Shaikh-Ubaid Shaikh-Ubaid requested a review from certik July 20, 2023 15:42
@anutosh491
Copy link
Collaborator

Good catch @anutosh491! Thank you. I fixed it in lpython.py: Decode ctypes strings before returning.

Yes , this seems to be the correct way to address this . Thank you

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

I think that this looks great.

Thank you.

This can also be used in the LLVM and C backends when linking. Right now we ignore it and one has to link by hand, which is fine.

@certik certik merged commit d41c25f into lcompilers:main Jul 20, 2023
9 checks passed
@Shaikh-Ubaid Shaikh-Ubaid deleted the fix_ccall_for_cpython branch July 20, 2023 17:24
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