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

Fix printing of unsigned integers #1896

Closed
Tracked by #1802
certik opened this issue Jun 13, 2023 · 4 comments · Fixed by #1897
Closed
Tracked by #1802

Fix printing of unsigned integers #1896

certik opened this issue Jun 13, 2023 · 4 comments · Fixed by #1897

Comments

@certik
Copy link
Contributor

certik commented Jun 13, 2023

def main():
    i: u8 = u8(255)
    print(i)

main()

This prints:

$ lpython a.py
-1

But should print 255.

@certik certik mentioned this issue Jun 13, 2023
25 tasks
@Smit-create
Copy link
Collaborator

So it needs a fix in the LLVM backend, and the fix basically requires keeping the value between [0, 2**(kind*8)). This should be handled at every assignment of unsigned integers and also while initializing. We need not worry about C backend as it will handle the same by itself.

This also needs some more fixes in LLVM because right now we treat i8 the same as u8 while i8 is unable to store 255. So we need to fix this by:

u_type = getIntType(max(a_kind*2, 8));

@certik
Copy link
Contributor Author

certik commented Jun 13, 2023

LLVM treats u8 and i8 the same. It's the same LLVM type. The u8(255) is exactly the same bit representation as i8(-1). So I think the only fix we have to do is in the visit_Print() to treat the u8 correctly.

certik added a commit to certik/lpython that referenced this issue Jun 13, 2023
@Smit-create
Copy link
Collaborator

The fix I was suggesting not just only applies to print. If we fix just print there can be many other failures for the same reason. See the following:

def main():
    siz: Const[u8] = u8(128)
    ai16: i16[siz] = empty(siz, dtype=int16)

main()

This fails in LLVM but not in C because in LLVM siz is -1.

@certik
Copy link
Contributor Author

certik commented Jun 13, 2023

The example you posted doesn't work: #1898. But assuming it "works", for now I would not allow unsigned integers in empty. So empty(siz) should give an error message in AST->ASR as well as in verify() that unsigned integer is not supported. In ASR this ends up being part of the "dimensions" and we should enforce in verify() that only signed integers are allowed. I made this #1899.

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 a pull request may close this issue.

2 participants