Skip to content

Throw Exception for wrong argtype in join method #2364

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.

Sign up for GitHub

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 12 commits into from
Oct 6, 2023
Merged

Throw Exception for wrong argtype in join method #2364

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a1f20e1
Throw Exception for wrong argtype in join method
Agent-Hellboy Oct 6, 2023
8cdf7cd
Remove SemanticError from test_str_01.py
Agent-Hellboy Oct 6, 2023
aba1128
Add reference for test handling join method
Agent-Hellboy Oct 6, 2023
74e08e5
Remove .stdout file
Agent-Hellboy Oct 6, 2023
4a4e40c
Run reference test
Agent-Hellboy Oct 6, 2023
5a22c59
Remove unnecessary files
Agent-Hellboy Oct 6, 2023
931f19d
Include update references
Agent-Hellboy Oct 6, 2023
2872a65
Update tests/errors/string_01.py
Agent-Hellboy Oct 6, 2023
da38f2f
Update test reference
Agent-Hellboy Oct 6, 2023
863faba
Remove .stdout file
Agent-Hellboy Oct 6, 2023
9cbc862
Add test for corner cases
Agent-Hellboy Oct 6, 2023
dcb18bd
Remove unused import
Agent-Hellboy Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions integration_tests/test_str_01.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from lpython import i32

def f():
x: str
x = "ok"
Expand Down Expand Up @@ -59,6 +61,13 @@ def test_str_join2():
res:str = a.join(p)
assert res == "a**b"

def test_str_join3():
a: str
a = "**-"
p:list[str] = ["a","b"]
res:str = a.join(p)
assert res == "a**-b"


def test_constant_str_subscript():
assert "abc"[2] == "c"
Expand All @@ -71,6 +80,8 @@ def check():
test_str_slice()
test_str_repeat()
test_str_join()
test_str_join2()
test_str_join3()
test_constant_str_subscript()

check()
15 changes: 10 additions & 5 deletions src/lpython/semantics/python_ast_to_asr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include <lpython/parser/parser.h>
#include <libasr/serialization.h>


namespace LCompilers::LPython {

namespace CastingUtil {
Expand Down Expand Up @@ -6669,15 +6668,21 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
throw SemanticError("str.join() takes one argument",
loc);
}
ASR::expr_t *arg_sub = args[0].m_value;
ASR::ttype_t *arg_sub_type = ASRUtils::expr_type(arg_sub);
if(!ASR::is_a<ASR::List_t>(*arg_sub_type)){
throw SemanticError("str.join() takes type list only",
loc);
}
Comment on lines +6671 to +6676
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change seems good. You can add an error reference test for this. Here is an example of how to add an error reference test 7c2bbac. You just add a test file in tests/errors. In the test file, you pass a type apart from list to str.join(). You enable this test by adding it to tests/tests.toml. Then you just need to update the reference tests using ./run_tests.py -u. This will add a new reference .stderr and .json files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I am still not checking the type of element inside the list which should be str, I checked grammar for asr but didn't get anything on how to iterate over list expression as it must be a list of str expressions. ast is getting generated perfectly but, I don't know how can I map ast nodes to asr nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did exact same thing you suggested, but CI is still failing , I must have missed something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Try updating refs using ./run_tests.py -u and stage all and commit the updated refs.

Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel Oct 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After implementing and committing the required changes, you can do the following to test everything on your machine:

$ ./build0.sh
$ ./build1.sh
$ ./run_tests.py # if this fails, check for the failure and do ./run_tests.py -u to update refs.
$ cd integration_tests && ./run_tests.py # Here you can do `./run_tests.py -b c` to test c backend, use `-h` option to view all the available options

If everything works, then you are good to go.

fn_call_name = "_lpython_str_join";
ASR::call_arg_t str_var;
str_var.loc = loc;
str_var.m_value = s_var;
ASR::call_arg_t passed_int;
passed_int.loc = loc;
passed_int.m_value = args[0].m_value;
ASR::call_arg_t list_of_str;
list_of_str.loc = loc;
list_of_str.m_value = args[0].m_value;
fn_args.push_back(al, str_var);
fn_args.push_back(al, passed_int);
fn_args.push_back(al, list_of_str);
} else if (attr_name == "find") {
if (args.size() != 1) {
throw SemanticError("str.find() takes one argument",
Expand Down
9 changes: 9 additions & 0 deletions tests/errors/string_02.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from lpython import i32

def test_wrong_argument_in_join():
x: str = "ab"
p: i32 = 1
res:str = x.join(p)
print(res)

test_wrong_argument_in_join()
13 changes: 13 additions & 0 deletions tests/reference/asr-string_02-499c9ff.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"basename": "asr-string_02-499c9ff",
"cmd": "lpython --show-asr --no-color {infile} -o {outfile}",
"infile": "tests/errors/string_02.py",
"infile_hash": "ed6511565e893791a4bd8ea0b4750817bab13cd6dc0731332127bf58",
"outfile": null,
"outfile_hash": null,
"stdout": null,
"stdout_hash": null,
"stderr": "asr-string_02-499c9ff.stderr",
"stderr_hash": "368ba74a1e0d6609f71e6f87f95bd0b6151420c81336e48a172cb613",
"returncode": 2
}
5 changes: 5 additions & 0 deletions tests/reference/asr-string_02-499c9ff.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
semantic error: str.join() takes type list only
--> tests/errors/string_02.py:6:15
|
6 | res:str = x.join(p)
| ^^^^^^^^^
1 change: 1 addition & 0 deletions tests/reference/asr-string_02-499c9ff.stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(Var 3 p)
4 changes: 4 additions & 0 deletions tests/tests.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,10 @@ asr = true
filename = "errors/string_01.py"
asr = true

[[test]]
filename = "errors/string_02.py"
asr = true

[[test]]
filename = "errors/structs_01.py"
asr = true
Expand Down