Mirror of forum.nim-lang.org

4891 :: rant about shr change

• adrianv (orginal) [2019-05-30T10:13:32+02:00] view original

  I don't know if I am overreacting, but just realized that

  Run

  has changed its meaning from unsigned shit to sign preserving shift. IMHO this breaks a lot of code in a very dangerous way. A more thoughtful solution would have been to introduce a sign preserving operator

  Run

  or something like that. Now we have a complete mess of old code in not well maintained but still working libraries with

  Run

  meaning a complete different thing than in new libraries. How can I rely on that a library is working correctly if I don't know what kind of

  Run

  the author had in mind when writing the library ? How do I use code with two libraries, one written with old

  Run

  and another with new

  Run

  ?

  But please show me that I am wrong !

---

• lscrd (orginal) [2019-05-30T18:11:54+02:00] view original

  I have not been able to reproduce this behavior. On my Manjaro Linux with the last development version (built with choosenim) shr is a logical right shift, not an arithmetic one. This is also the case with the last stable version.

  And there is already an arithmetic right shift in the system module. This a function (not an operator) named ashr.

---

• Stefan_Salewski (orginal) [2019-05-30T18:36:28+02:00] view original

  I have not been able to reproduce

  Well, it is very confusing. I looked in 019.9 docs this morning, and indeed it was said that shr behaves as ashr. But example was stlll like shr, and additional a separate ashr was mentioned. So maybe just a bug in docs. Otherwise for sure adrianv would be right, as it would be a drastic change, and name shr is bad for arithmetic shift.

---

• lscrd (orginal) [2019-05-30T23:16:51+02:00] view original

  Yes, there is indeed a change in the documentation between 0.19.6 and 0.19.9.

  Personally, for signed integers I would find more consistent to use an arithmetic shift to the right. With a logical shift, -2 shr 1 is equal to 9223372036854775807 on a 64 bits machine which may seem odd. We would rather expect to get -1.

  In C, the shift to the right is arithmetic for signed integers, logical for unsigned integers, which makes sense. And to get a logical shift for a signed integer, you have only to cast it to an unsigned int.

  Now, as shr has always been a logical shift, I agree that it would be a questionable change which could break code in some subtile ways.

---

• Araq (orginal) [2019-05-31T19:51:27+02:00] view original

  Compile with old code with -d:nimOldShiftRight to find all usages of shr. Maybe we need a better transition period or a better solution altogether. I'm open to suggestions. However, the old unsigned shr behaviour for signed numbers also was seen as very error prone.

---

• adrianv (orginal) [2019-06-01T10:19:02+02:00] view original

  I agree that shr on signed int is error prone, but that is how the things are now. To have a save transition I would suggest to have 2 operators shr and another one ashr or sar or whatever with signed shift behavior. We should then deprecate shr for signed int and in the future make it an error to use it with signed int. If someone needs pure binary shift, (s)he should cast the signed int to uint

---

• Araq (orginal) [2019-06-01T13:48:01+02:00] view original

  We already have ashr. We considered adding ushr for the old "unsigned shr" but then you might as well cast the numbers to unsigned before the shr operation so it wouldn't gain us much.

---

• adrianv (orginal) [2019-06-02T11:02:08+02:00] view original

  now I am completely confused. When there is a ashr (which I have never noticed), why is there the need to change the behavior of shr for signed int? I can't see the point.

---

• Araq (orginal) [2019-06-02T17:38:52+02:00] view original

  Because we can then deprecate ashr.

---

• GordonBGood (orginal) [2019-06-04T05:39:07+02:00] view original

  Because we can then deprecate ashr.

  @Araq, it would be great to get rid of that!

  However, as per the latest documentation for shr:

  ...filling vacant bit positions with the sign bit.

  This would mean that a shr of a "minus one" of any type should do the same as a no-op whereas a shr of any unsigned integers should zero fill as there is no "sign bit".

  However, at least for version 0.19.9, it doesn't do as advertised, with the first group using unsigned behaving as expected, the second group passing when they shouldn't and the third group failing when they should pass:

  doAssert 0xFF'u8 shr 1 == 0x7F'u8
  doAssert 0xFFFF'u16 shr 1 == 0x7FFF'u16
  doAssert 0xFFFFFFFF'u32 shr 1 == 0x7FFFFFFF'u32
  doAssert 0xFFFFFFFFFFFFFFFF'u64 shr 1 == 0x7FFFFFFFFFFFFFFF'u64
  doAssert 0xFFFFFFFFFFFFFFFF'u shr 1 == 0x7FFFFFFFFFFFFFFF'u
  
  doAssert -1'i8 shr 1 == 0x7F'i8
  doAssert -1'i16 shr 1 == 0x7FFF'i16
  doAssert -1'i32 shr 1 == 0x7FFFFFFF'i32
  doAssert -1'i64 shr 1 == 0x7FFFFFFFFFFFFFFF'i64
  doAssert -1 shr 1 == 0x7FFFFFFFFFFFFFFF
  
  doAssert -1'i8 shr 1 == -1'i8
  doAssert -1'i16 shr 1 == -1'i16
  doAssert -1'i32 shr 1 == -1'i32
  doAssert -1'i64 shr 1 == -1'i64
  doAssert -1 shr 1 == -1
  Run

  As per the following Wandbox: https://wandbox.org/permlink/jwX8vctgMmTxoTVN

  When this works as advertised (including examples showing these edge cases of the most significant bit set) it will be great, as it then works the same as logical languages like F# where the behaviour depends on the unsigned or not integer type.

---