Skip to content

Enabling c backend testing for integration_tests/test_c_interop_01.py #574

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 19 commits into from
Jun 12, 2022

Conversation

czgdp1807
Copy link
Collaborator

No description provided.

@czgdp1807 czgdp1807 added the c Label for C language related changes label Jun 10, 2022
@czgdp1807 czgdp1807 marked this pull request as draft June 10, 2022 13:47
…ich are not present in lfortran_intrinsics.h

2. Implement only non-empty functions, if not done then this leads to duplicate declaration
@czgdp1807
Copy link
Collaborator Author

The file, test_c_interop_01 compiles but I think there are a bunch of features which need to be implemented,

  1. Initialising variables at declaration. See the value of pi is printed as 0.
  2. abs sometimes doesn't work correctly. It fails even though values are correct. I will have to see what's going on.
[ 33%] Generating test_c_interop_01.c
[ 66%] Building C object CMakeFiles/test_c_interop_01.dir/test_c_interop_01.c.o
[100%] Linking C executable test_c_interop_01
[100%] Built target test_c_interop_01
(lp) 19:47:54:~/lpython_project/lpython/integration_tests % ./test_c_interop_01
0.000000
0.000000
1.000000
ASSERT failed: /Users/czgdp1807/lpython_project/lpython/integration_tests/test_c_interop_01.c
function test_c_callbacks(), line number 52 at 
__lpython_overloaded_0__abs(_lfortran_dsin(pi/(float)(2)) - (float)(1)) < 1.000000e-12

@certik
Copy link
Contributor

certik commented Jun 11, 2022

I think it's fine, but didn't have time to study carefully. Tests should be fixed first.

@czgdp1807
Copy link
Collaborator Author

Tests should be fixed first.

Will do that once I convert this to ready mode. Meanwhile if you have any thoughts/comments/objections on the changes, then feel free to let me know.

@certik
Copy link
Contributor

certik commented Jun 11, 2022

I think the changes are good, thanks!

@czgdp1807
Copy link
Collaborator Author

Tests should pass. I hope. 🤞.

@czgdp1807 czgdp1807 marked this pull request as ready for review June 11, 2022 16:27
@czgdp1807
Copy link
Collaborator Author

Tests pass. This is ready.

@czgdp1807 czgdp1807 requested a review from certik June 11, 2022 17:07
@certik
Copy link
Contributor

certik commented Jun 12, 2022

I don't like commenting out tests, as we need them for LLVM and pure Python too. But fortunately I fixed most of these issues in main already, so I got the asserts working in this PR.

@certik certik enabled auto-merge June 12, 2022 04:35
@certik certik merged commit 6dbb04a into lcompilers:main Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c Label for C language related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants