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

Implement str to int conversion #2599

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kmr-srbh
Copy link
Contributor

@kmr-srbh kmr-srbh commented Mar 11, 2024

Resolves #2554

Implement basic string to integer conversion using CharacterToInteger casting, which I believe is a part of LFortran, but was not implemented here in LPython.

Working

from lpython import i8, i16, i32, i64

int8: i8 = i8("100")
int16: i16 = i16("100")
int32: i32 = i32("100")
int64: i64 = i64("100")

print("int8:", int8)
print("int16:", int16)
print("int32:", int32)
print("int64:", int64)
print()
print("int8 + i8(10):", int8 + i8(10))
print("int16 * i16(2):", int16 * i16(2))
print("int32 - 50:", int32 - 50)
print("int64 / i64(2):", int64 / i64(2))
(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
int8: 100
int16: 100
int32: 100
int64: 100

int8 + i8(10): 110
int16 * i16(2): 200
int32 - 50: 50
int64 / i64(2): 5.00000000000000000e+01

ASR

                            int32:
                                (Variable
                                    2
                                    int32
                                    []
                                    Local
                                    (Cast
                                        (StringConstant
                                            "100"
                                            (Character 1 3 ())
                                        )
                                        CharacterToInteger
                                        (Integer 4)
                                        (IntegerConstant 100 (Integer 4))
                                    )
                                    (IntegerConstant 100 (Integer 4))
                                    Default
                                    (Integer 4)
                                    ()
                                    Source
                                    Public
                                    Required
                                    .false.
                                ),
                            int64:
                                (Variable
                                    2
                                    int64
                                    []
                                    Local
                                    (Cast
                                        (StringConstant
                                            "100"
                                            (Character 1 3 ())
                                        )
                                        CharacterToInteger
                                        (Integer 8)
                                        (IntegerConstant 100 (Integer 8))
                                    )
                                    (IntegerConstant 100 (Integer 8))
                                    Default
                                    (Integer 8)
                                    ()
                                    Source
                                    Public
                                    Required
                                    .false.
                                ),

Tasks

  • Throw a semantic error when an invalid literal is passed. While this can be handled while casting, it is not very clean to do it there. I request guidance on where the error should be thrown.
  • Throw an error when an integer larger or smaller than the upper and lower bounds is passed as a valid string. I request guidance on where it should be handled.
  • Handle integers with other bases. I doubt whether the types i8, i16, etc even take a 2nd argument. Even when several arguments are passed, only the 1st is dealt with and errors related to the same are thrown.

@kmr-srbh kmr-srbh marked this pull request as draft March 14, 2024 03:48
@Shaikh-Ubaid
Copy link
Collaborator

Please mark this as "Ready for review" when ready. If you have any doubt, please ask.

@kmr-srbh
Copy link
Contributor Author

kmr-srbh commented Mar 17, 2024

@Shaikh-Ubaid I do have some doubts. They are related to throwing errors.

  1. Where should the case of an invalid string literal passed to the numeric types be handled? Before coming to the casting part, we cannot be sure that the passed value is an integer enclosed inside a string. This can be dealt with before we call the cast_helper, but then again, we would be repeating the same code as that in the casting. Handling the same during casting is not clean in my view.
  2. This is very important. We will have to handle the case of an integer larger than the storage type passed to the variable. stoi and stol handle that for us for i32 and i64, but for the smaller ones it leads to the overflow.

My chief concern is that while both of these can be done while casting, it is not clean, and probably not supported there too.

@Shaikh-Ubaid
Copy link
Collaborator

Sharing my thoughts on this:

  • I think string to integer is more of a conversion rather than a casting operation.
  • So for now let's not use i32, i16, ... to cast string to int. Let's reserve them only for casting from other integers and floats to integers
  • Now for conversion to string, let's use the int() function

I believe the above approach solves all your concerns.

@Shaikh-Ubaid
Copy link
Collaborator

Also see #2554 (comment).

@kmr-srbh
Copy link
Contributor Author

kmr-srbh commented Mar 17, 2024

Now for conversion to string, let's use the int() function

I think there is a catch. @certik said that int() will mean an arbitrary precision integer in future for LPython. Considering the case of a hexadecimal or an octal number, the result of something like i32(int("0x101100", 16))" will cause an error as the value is too large to fit in an i32.

Supporting your idea and building upon it, I think we need to not just cast a string to an integer, but convert it. Please let me know your thoughts on this. :)

But again, we can surely build on top of the support we have for int() now. Let's work out a plan for that.

@Shaikh-Ubaid
Copy link
Collaborator

int() will mean an arbitrary precision integer in future for LPython.

I think the type int would mean an arbitrary precision integer. For example, x: int. I think for now it is safe to use int() for string to int conversion.

Considering the case of a hexadecimal or an octal number, the result of something like i32(int("0x101100", 16))" will cause an error as the value is too large to fit in an i32.

Let's just focus on normal/decimal (base 10) integers for now. Once that is robustly supported, we can try supporting integers of other bases.

@Shaikh-Ubaid
Copy link
Collaborator

But again, we can surely build on top of the support we have for int() now. Let's work out a plan for that.

Sure, go ahead and make a plan. Figure out what is left to be supported for int() or what fails and you can work on fixing it.

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 this pull request may close these issues.

String to int conversion in ASR
2 participants