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

Rename ConvertToLong to ConvertToInt #1403

Merged
merged 5 commits into from
Apr 18, 2022

Conversation

BCSharp
Copy link
Member

@BCSharp BCSharp commented Apr 17, 2022

Follow-up on #1329 (issue #52).

Since __long__ is gone, to avoid confusion, XxxConvertToLong is renamed to XxxConvertToInt, while the old XxxConvertToInt is gone. Plus some cleanup. A couple of functional changes are fixes:

  • If __int__ returns a BigInteger instance, a conversion call site with return type Int32 succeeds or throws OverflowException if the value does not fit in 32 bits. Previously it was always throwing TypeErrorException with a rather cryptic message: __int__ returned non-int (int), regardless of the value.
  • If __complex__ returns non-complex, TypeErrorException with a proper message is returned __complex__ returned non-complex (xxx) rather than InvalidCastException.

public static object? ConvertIntToInt32(object? value) {
return value switch {
null => null,
int i => i,
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably insignificant optimization, but maybe return value here? I guess the same applies to ConvertIntToBigInt.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it and took the lead from ConvertToPythonPrimitive (a few lines above), which does more of this kind of stuff, presumably for readability (?).

OK to optimize it.

Copy link
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@BCSharp BCSharp merged commit c55190b into IronLanguages:master Apr 18, 2022
@BCSharp BCSharp deleted the convert_to_long branch April 18, 2022 03:50
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.

None yet

2 participants