Add isspace API in str by Agent-Hellboy · Pull Request #2373 · lcompilers/lpython

Agent-Hellboy · 2023-10-08T04:41:35Z

Agent-Hellboy · 2023-10-08T05:15:16Z

is the ci check disabled because of the conflicting files?
I forgot to do a rebase before running ./run_test.py -u what should I do?

Thirumalai-Shaktivel · 2023-10-08T06:22:45Z

is the ci check disabled because of the conflicting files?

Yup!

I forgot to do a rebase before running ./run_test.py -u what should I do?

$ git checkout main 
$ git pull lpython main # where `lpython` is the remote name of the lpython main repository
$ git checkout add_isspace_api
$ git merge main
   # resolve conflicts if any
$ git reset main
   # Now, all the changes you made will be ready to be staged
$ git add . # `.` represents the file you want to commit
$ git commit -m ".. message" # Commit all the files 
$ git push -f origin add_isspace_api # where `origin` is the remote name for your lpython forked repository

For more details on merging and rebasing, go through the page: https://github.com/lcompilers/lpython/blob/main/doc/src/rebasing.md

Agent-Hellboy · 2023-10-08T06:55:27Z

Thanks, @Thirumalai-Shaktivel, what should I do for the below error?

compiler_tester.tester.RunException: Testing with reference output failed.
complex1.py * asr
The JSON metadata differs against reference results
Reference JSON: tests/reference/asr-complex1-f26c460.json
Output JSON:    tests/output/asr-complex1-f26c460.json
Omitting 6 identical items
Differing items:
{'returncode': 0} != {'returncode': 2}
{'stderr_hash': None} != {'stderr_hash': '850f4cbe87b0a7ae81[38](https://github.com/lcompilers/lpython/actions/runs/6446076488/job/17500797616?pr=2373#step:6:39)fbfb71f94b82843abf09e50752159f900292'}
{'stdout_hash': 'd2492faf2c4817c79e63e63c41265783d5cc209b149ea270194804[40](https://github.com/lcompilers/lpython/actions/runs/6446076488/job/17500797616?pr=2373#step:6:41)'} != {'stdout_hash': None}
{'stderr': None} != {'stderr': 'asr-complex1-f26c[46](https://github.com/lcompilers/lpython/actions/runs/6446076488/job/17500797616?pr=2373#step:6:47)0.stderr'}
{'stdout': 'asr-complex1-f26c460.stdout'} != {'stdout': None}No reference stdout_hash available for complex1.py

Do reference tests get generated on the fly, do I need to delete all reference tests and then run ./run_tests.py -u?

Thirumalai-Shaktivel · 2023-10-08T06:57:35Z

Don't do it all the time, but for this PR you can do rm tests/reference/* and then do ./run_tests.py -u

Thirumalai-Shaktivel · 2023-10-08T07:15:53Z

I'm sorry, I forgot to add the following command above:
After committing the changes, you have to make a clean build and then update tests,

$ git clean -dfx
$ ./build.sh
$ ./run_tests.py # Use `-u` to update tests.

Thirumalai-Shaktivel · 2023-10-08T07:30:03Z

tests/reference/asr-array_01_decl-39cf894.stderr

@@ -0,0 +1,17 @@
+warning: The module 'numpy' located in $DIR/src/bin/../runtime/lpython_intrinsic_numpy.py cannot be loaded
+ --> tests/../integration_tests/array_01_decl.py:2:1


All the newly created *.stderr files needs to be removed.

For that,

you can do rm tests/reference/* and then do ./run_tests.py -u

src/runtime/lpython_builtin.py

Thirumalai-Shaktivel

Looks great, thank you!

ubaidsk · 2023-10-08T07:46:37Z

@Agent-Hellboy The CI seems to fail currently.

Agent-Hellboy · 2023-10-08T07:53:52Z

@Agent-Hellboy The CI seems to fail currently.

yes,

/home/runner/work/lpython/lpython/integration_tests/_lpython-tmp-test-c/test_str_attributes.c:117:6: warning: conflicting types for built-in function ‘isspace’; expected ‘int(int)’ [-Wbuiltin-declaration-mismatch]
  117 | void isspace();
      |      ^~~~~~~
/home/runner/work/lpython/lpython/integration_tests/_lpython-tmp-test-c/test_str_attributes.c:9:1: note: ‘isspace’ is declared in header ‘<ctype.h>’
    8 | #include <lfortran_intrinsics.h>
  +++ |+#include <ctype.h>
    9 | 
/home/runner/work/lpython/lpython/integration_tests/_lpython-tmp-test-c/test_str_attributes.c: In function ‘__lpython_overloaded_0___lpython_str_isspace’:
/home/runner/work/lpython/lpython/integration_tests/_lpython-tmp-test-c/test_str_attributes.c:1991:167: error: \u000b is not a valid universal character
 1991 |         if (strcmp(ch, " ")  !=  0 && strcmp(ch, "\t")  !=  0 && strcmp(ch, "\n")  !=  0 && strcmp(ch, "\r")  !=  0 && strcmp(ch, "\\f")  !=  0 && strcmp(ch, "\u000b")  !=  0) {
      |

is one of the backend treating \v as \u000b ?

ubaidsk · 2023-10-08T07:58:07Z

is one of the backend treating \v to \u000b ?

It is the C backend. You can tell it from _lpython-tmp-test-c (the c at the end). The specific test that fails is integration_tests/test_str_attributes.py. Try looking into the backend asr_to_c.cpp or asr_to_c_cpp.cpp. You can simply test this specific test case using ./src/bin/lpython integration_tests/test_str_attributes.py --backend=c.

You can construct a simple/minimum example using the \v and see what the C backend generates for it. Then using this simple example, we can debug the C backend.

Agent-Hellboy · 2023-10-08T08:05:00Z

You can construct a simple/minimum example using the \v and see what the C backend generates for it. Then using this simple example, we can debug the C backend.

The \v escape sequence is part of the standard C escape sequences
https://en.wikipedia.org/wiki/Escape_sequences_in_C
don't know why it is converting \v to \u000b

Agent-Hellboy · 2023-10-08T08:17:31Z

I have used '\x0B'for now, created an issue #2375

Co-authored-by: Shaikh Ubaid <[email protected]>

certik

I think that this is good, I left some comments.

src/lpython/semantics/python_ast_to_asr.cpp

Co-authored-by: Ondřej Čertík <[email protected]>

src/lpython/semantics/python_ast_to_asr.cpp

certik · 2023-10-08T12:49:48Z

src/runtime/lpython_builtin.py

            return False
    return True

+@overload


Does this work:

Suggested change

@overload

I don't know what overload does. from the code, it seems it adds attributes to it.

The @overload allows you to use the same function name with different arguments like int and float. In this case there is only one function, so overload is not needed.

Yup, we have to remove @overload for many functions. I got confused with the actual meaning of overload, as it was used everywhere.

src/lpython/semantics/python_ast_to_asr.cpp

Co-authored-by: Ondřej Čertík <[email protected]>

certik · 2023-10-08T12:56:03Z

@Agent-Hellboy why don't you fix the comment per my post above and then we can merge it.

Agent-Hellboy · 2023-10-08T12:56:28Z

@Agent-Hellboy why don't you fix the comment per my post above and then we can merge it.

sure

src/lpython/semantics/python_ast_to_asr.cpp

ubaidsk

Amazing work @Agent-Hellboy! Thank you so much for this!

Fix merge conflicts

f40f524

Agent-Hellboy force-pushed the add_isspace_api branch from 5e75fa1 to f40f524 Compare October 8, 2023 06:34

Update reference tests

a468227

certik marked this pull request as draft October 8, 2023 06:53

Agent-Hellboy added 3 commits October 8, 2023 12:29

Fix test references

18b2f70

Delete test references

b89419d

Add test reference

08aa19a

Clean test references

dad38f9

Thirumalai-Shaktivel reviewed Oct 8, 2023

View reviewed changes

Delete unnecessary test references

2642039

ubaidsk reviewed Oct 8, 2023

View reviewed changes

src/runtime/lpython_builtin.py Outdated Show resolved Hide resolved

Resolve symbolic error

273844c

Thirumalai-Shaktivel approved these changes Oct 8, 2023

View reviewed changes

Fix CI

c4af949

ubaidsk mentioned this pull request Oct 8, 2023

why \v is getting converted into \u000b during asr_to_c conversion #2375

Closed

Agent-Hellboy and others added 2 commits October 8, 2023 14:00

Add a new escape sequence

c576fa4

Update src/runtime/lpython_builtin.py

a9ac4af

Co-authored-by: Shaikh Ubaid <[email protected]>

Agent-Hellboy marked this pull request as ready for review October 8, 2023 09:06

certik approved these changes Oct 8, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

Update src/lpython/semantics/python_ast_to_asr.cpp

24ec976

Co-authored-by: Ondřej Čertík <[email protected]>

Agent-Hellboy and others added 2 commits October 8, 2023 17:32

Update src/lpython/semantics/python_ast_to_asr.cpp

96c069d

Co-authored-by: Ondřej Čertík <[email protected]>

Update src/lpython/semantics/python_ast_to_asr.cpp

8fa3b8d

Co-authored-by: Ondřej Čertík <[email protected]>

certik reviewed Oct 8, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

certik reviewed Oct 8, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

Agent-Hellboy and others added 2 commits October 8, 2023 18:24

Update src/lpython/semantics/python_ast_to_asr.cpp

ceafe99

Co-authored-by: Ondřej Čertík <[email protected]>

Update src/runtime/lpython_builtin.py

ce6917e

Co-authored-by: Ondřej Čertík <[email protected]>

Fix comment

c7fcfce

certik reviewed Oct 8, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

Update src/lpython/semantics/python_ast_to_asr.cpp

58f8dc7

certik approved these changes Oct 8, 2023

View reviewed changes

certik enabled auto-merge (squash) October 8, 2023 13:01

certik merged commit 87e7caa into lcompilers:main Oct 8, 2023

ubaidsk reviewed Oct 8, 2023

View reviewed changes

Agent-Hellboy added a commit to Agent-Hellboy/lpython that referenced this pull request Oct 9, 2023

Add isspace API in str (lcompilers#2373)

223ac46

		@@ -0,0 +1,17 @@
		warning: The module 'numpy' located in $DIR/src/bin/../runtime/lpython_intrinsic_numpy.py cannot be loaded
		--> tests/../integration_tests/array_01_decl.py:2:1

Comments

Conversation

Agent-Hellboy commented Oct 8, 2023 • edited by ubaidsk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Agent-Hellboy commented Oct 8, 2023

Uh oh!

Thirumalai-Shaktivel commented Oct 8, 2023

Uh oh!

Agent-Hellboy commented Oct 8, 2023

Uh oh!

Thirumalai-Shaktivel commented Oct 8, 2023

Uh oh!

Thirumalai-Shaktivel commented Oct 8, 2023

Uh oh!

Thirumalai-Shaktivel Oct 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

Uh oh!

ubaidsk commented Oct 8, 2023

Uh oh!

Agent-Hellboy commented Oct 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ubaidsk commented Oct 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Agent-Hellboy commented Oct 8, 2023

Uh oh!

Agent-Hellboy commented Oct 8, 2023

Uh oh!

certik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

certik Oct 8, 2023

Choose a reason for hiding this comment

Uh oh!

Agent-Hellboy Oct 8, 2023

Choose a reason for hiding this comment

Uh oh!

certik Oct 8, 2023

Choose a reason for hiding this comment

Uh oh!

Thirumalai-Shaktivel Oct 8, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

certik commented Oct 8, 2023

Uh oh!

Agent-Hellboy commented Oct 8, 2023

Uh oh!

Uh oh!

ubaidsk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Agent-Hellboy commented Oct 8, 2023 •

edited by ubaidsk

Loading

Thirumalai-Shaktivel Oct 8, 2023 •

edited

Loading

Agent-Hellboy commented Oct 8, 2023 •

edited

Loading

ubaidsk commented Oct 8, 2023 •

edited

Loading