Add isspace API in str #2373

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!

Shaikh-Ubaid · 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 ?

Shaikh-Ubaid · 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 <shaikhubaid769@gmail.com>

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 <ondrej@certik.us>

src/lpython/semantics/python_ast_to_asr.cpp

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

src/runtime/lpython_builtin.py

@@ -858,6 +858,15 @@ def _lpython_str_isascii(s: str) -> bool:
            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 <ondrej@certik.us>

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

Shaikh-Ubaid

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

Shaikh-Ubaid 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

Shaikh-Ubaid 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 <shaikhubaid769@gmail.com>

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 <ondrej@certik.us>

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 <ondrej@certik.us>

Update src/lpython/semantics/python_ast_to_asr.cpp

8fa3b8d

Co-authored-by: Ondřej Čertík <ondrej@certik.us>

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 <ondrej@certik.us>

Update src/runtime/lpython_builtin.py

ce6917e

Co-authored-by: Ondřej Čertík <ondrej@certik.us>

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
13 checks passed

Shaikh-Ubaid 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add isspace API in str #2373

Add isspace API in str #2373

Agent-Hellboy commented Oct 8, 2023 •

edited by Shaikh-Ubaid

Agent-Hellboy commented Oct 8, 2023

Thirumalai-Shaktivel commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023

Thirumalai-Shaktivel commented Oct 8, 2023

Thirumalai-Shaktivel commented Oct 8, 2023

Thirumalai-Shaktivel Oct 8, 2023 •

edited

Thirumalai-Shaktivel left a comment

Shaikh-Ubaid commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023 •

edited

Shaikh-Ubaid commented Oct 8, 2023 •

edited

Agent-Hellboy commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023

certik left a comment

certik Oct 8, 2023

Agent-Hellboy Oct 8, 2023

certik Oct 8, 2023

Thirumalai-Shaktivel Oct 8, 2023

certik commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023

Shaikh-Ubaid left a comment

		@@ -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

Add isspace API in str #2373

Add isspace API in str #2373

Conversation

Agent-Hellboy commented Oct 8, 2023 • edited by Shaikh-Ubaid

Agent-Hellboy commented Oct 8, 2023

Thirumalai-Shaktivel commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023

Thirumalai-Shaktivel commented Oct 8, 2023

Thirumalai-Shaktivel commented Oct 8, 2023

Thirumalai-Shaktivel Oct 8, 2023 • edited

Choose a reason for hiding this comment

Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

Shaikh-Ubaid commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023 • edited

Shaikh-Ubaid commented Oct 8, 2023 • edited

Agent-Hellboy commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023

certik left a comment

Choose a reason for hiding this comment

certik Oct 8, 2023

Choose a reason for hiding this comment

Agent-Hellboy Oct 8, 2023

Choose a reason for hiding this comment

certik Oct 8, 2023

Choose a reason for hiding this comment

Thirumalai-Shaktivel Oct 8, 2023

Choose a reason for hiding this comment

certik commented Oct 8, 2023

Agent-Hellboy commented Oct 8, 2023

Shaikh-Ubaid left a comment

Choose a reason for hiding this comment

Agent-Hellboy commented Oct 8, 2023 •

edited by Shaikh-Ubaid

Thirumalai-Shaktivel Oct 8, 2023 •

edited

Agent-Hellboy commented Oct 8, 2023 •

edited

Shaikh-Ubaid commented Oct 8, 2023 •

edited