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

int int32 int64 are all distinct types, but float and float64 are not #5875

Closed
krux02 opened this issue May 23, 2017 · 6 comments
Closed

int int32 int64 are all distinct types, but float and float64 are not #5875

krux02 opened this issue May 23, 2017 · 6 comments

Comments

@krux02
Copy link
Contributor

krux02 commented May 23, 2017

proc foo(arg: int): void =
  echo "int"

proc foo(arg: int32): void =
  echo "int32"

proc foo(arg: int64): void =
  echo "int64"

# these three are all fine
foo(123)
foo(123'i32)
foo(123'i64)

proc bar(arg: float): void =
  echo "float"

proc bar(arg: float32): void =
  echo "float32"

proc bar(arg: float64): void =   # Error: redefinition of 'foo'
  echo "float64"

bar(123.45)
bar(123.45'f32)
bar(123.45'i64)

cause is at the top of system.nim

type
  int* {.magic: Int.} ## default integer type; bitwidth depends on
                      ## architecture, but is always the same as a pointer
  int8* {.magic: Int8.} ## signed 8 bit integer type
  int16* {.magic: Int16.} ## signed 16 bit integer type
  int32* {.magic: Int32.} ## signed 32 bit integer type
  int64* {.magic: Int64.} ## signed 64 bit integer type
  uint* {.magic: UInt.} ## unsigned default integer type
  uint8* {.magic: UInt8.} ## unsigned 8 bit integer type
  uint16* {.magic: UInt16.} ## unsigned 16 bit integer type
  uint32* {.magic: UInt32.} ## unsigned 32 bit integer type
  uint64* {.magic: UInt64.} ## unsigned 64 bit integer type
  float* {.magic: Float.} ## default floating point type
  float32* {.magic: Float32.} ## 32 bit floating point type
  float64* {.magic: Float.} ## 64 bit floating point type    <------ HERE magic is Float not Float64

that is also inconsistent with the macros library that does indeed distinguishes between float and float64 literals.

@Araq
Copy link
Member

Araq commented May 29, 2017

This is not a bug, the spec was changed so that Nim only has 2 float types to simplify some math libraries.

@krux02
Copy link
Contributor Author

krux02 commented May 29, 2017

I don't have a problem that there are only two float types, the problem I have is, that float to float32, float64 is not the same as int is to int32, int64. That is in my opinion just unnecessarily confusing.

Wouldn't this be a better alternative?

type
  int8* {.magic: Int8.} ## signed 8 bit integer type
  int16* {.magic: Int16.} ## signed 16 bit integer type
  int32* {.magic: Int32.} ## signed 32 bit integer type
  int64* {.magic: Int64.} ## signed 64 bit integer type
  uint8* {.magic: UInt8.} ## unsigned 8 bit integer type
  uint16* {.magic: UInt16.} ## unsigned 16 bit integer type
  uint32* {.magic: UInt32.} ## unsigned 32 bit integer type
  uint64* {.magic: UInt64.} ## unsigned 64 bit integer type
  float32* {.magic: Float32.} ## 32 bit floating point type
  float64* {.magic: Float64.} ## 64 bit floating point type

when sizeof(pointer) == 8:
  type
    int*   = int64 ## default integer type; bitwidth depends on architecture, but is always the same as a pointer
    uint*  = uint64
    float* = float64
elif sizeof(pointer) == 4:
  type
    int*   = int32
    uint*  = uint32
    float* = float32
elif sizeof(pointer) == 2:
  type
    int*   = int16
    uint*  = uint16
    float* = float32
else:
  static:
    assert false

@Araq
Copy link
Member

Araq commented May 31, 2017

Yes and no. Your solution means 'tyInt' is not builtin but just an alias (I like the idea but who knows what breaks it). 'float's definition should NOT depend on the pointer size though there is no logical connection here and in fact, it makes things worse when an 'int32' cannot even fit in a 'float' without precision/truncation problems.

@krux02
Copy link
Contributor Author

krux02 commented May 31, 2017

Yes removing 'tyInt' is a breaking change. But I don't think it's a problematic one. For example the example above would complain:

proc foo(arg: int): void =
  echo "int"

proc foo(arg: int32): void =
  echo "int32"

proc foo(arg: int64): void =  # <--- HERE it would complain about redefinition of foo(int64)
  echo "int64"

Eventually it would make things simpler and less ugly, because fewer types need to be taken care of. I think everything people actually have to do, is to remove branches, where int64 and int are treated as distinct types. If they actually do it, it just means to remove code, not add extra steps. So in my opinion it would be a forced code improvement.

If I remembor correctly I once had the case where I implemented an operation for both int32 and int64, and then it bothered me that ```int`` was not simply defined as an alias

proc foo(arg: int32): int32 {. importc: "foo_f" .}
  ## do foo on an ``int32``

proc foo(arg: int64): int64 {. importc: "foo_d" .}
  ## do foo on an ``int64``

# now, because ``int`` is not simply an alias I do need to do this weird thing

proc foo(arg: int): int =
  when sizeof(int) == sizeof(int32):
    arg.int32.foo.int
  elif sizeof(int) == sizeof(int64):
    arg.int64.foo.int

With int as being an alias, I would be forced to remove the wrapper for the int type.

@Araq
Copy link
Member

Araq commented May 31, 2017

Eventually it would make things simpler and less ugly, because fewer types need to be taken care of. I think everything people actually have to do, is to remove branches, where int64 and int are treated as distinct types. If they actually do it, it just means to remove code, not add extra steps. So in my opinion it would be a forced code improvement.

Agreed. But it will cause huge problems with backwards compatibility and Nim has the notion of an "int literal" type so that e.g. i16 + int literal == i16. Expect at least months of work to get these details right. -.-

@krux02
Copy link
Contributor Author

krux02 commented May 31, 2017

Well I just did a few tests and did the following observation:

import typetraits

proc main() =
  var i1 = 1'i8 +  int(33); echo i1, "\t", i1.type.name #   34	int  # OK
  var i2 = 1'i8 +      33 ; echo i2, "\t", i2.type.name #   34	int8 # this stays int8
  var i3 = 1'i8 +     127 ; echo i3, "\t", i3.type.name # -128	int8 # overflow without error
  var i4 = 0'i8 +     128 ; echo i4, "\t", i4.type.name #  128	int  # now it becomes an int
  var i5 = 1'i8 + int8(33); echo i5, "\t", i5.type.name #   34	int8 # OK
  var i6 =             33 ; echo i6, "\t", i6.type.name #   33	int  # 33 alone is an int

  var a = 1'i8
  var b = 127'i8
  echo a + b  # Error: OverflowError
main()

My idea at the moment is to set the type of an int literal to be explicity of type int which is then just an alias. Then the output of the program on 64 bit system would be the following:

  var i1 = 1'i8 +  int(33); echo i1, "\t", i1.type.name #   34	int64
  var i2 = 1'i8 +      33 ; echo i2, "\t", i2.type.name #   34	int64
  var i3 = 1'i8 +     127 ; echo i3, "\t", i3.type.name #  128	int64
  var i4 = 0'i8 +     128 ; echo i4, "\t", i4.type.name #  128	int64
  var i5 = 1'i8 + int8(33); echo i5, "\t", i5.type.name #   34	int8
  var i6 =             33 ; echo i6, "\t", i6.type.name #   33	int64

Yea breaking change again :(, but simpler rules and things would be again more consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants