Array type parsing error/improvment?

1

I had an array I initialized with one value like this.

arry = [1] of UInt64

However, "1" was used as Int32 and created math errors with UInt64s.
I fixed the “problem” like this:

arry = [1u64] of UInt64

But this seems totally redundant and, IMHO, should be unnecessary.
I think the first case should compile the "1" as an UInt64.
Otherwise at minimum, the compiler should give an error/warning to let the programmer know of this type conflict (if it doesn’t automatically convert it).

2

I guess this pretty much is https://github.com/crystal-lang/crystal/issues/9565

FWIW arry = [1u64] is valid and does what you expect.

3

Yes, it’s the exact issue. Here’s why I think it’s important to "fix" it.

I’ve been using Crystal long enough now to find what the source of this type of problem could be. But people new to Crystal, or coming from Ruby (or other dynamic languages) might not have the experience to intuitively know what the source of a problem like this is.

I create the array values in one function, and pass them to another that does the math with them. So the math type conflict shows up in one function, but its cause was in a totally different one.

This problem didn’t show up at compile time, it was a runtime error!
Except for the first value, every other value in the array worked. This could drive a new/novice/unsuspecting programmer maaad! :fearful:

Anyway, I’m glad this issue was raised previously and I hope you implement auto-casting, but at least make the compiler throw a type conflict error/warning until then.

EDIT: It was a compile time error, but I discovered the fix at runtime.

4

@jzakiya Could you provide the code that triggers the error? [1] of UInt64 compiles just fine.

5

The first functions is where the array is created, the the second is where it’s used.

def rangeres(modpn, n)
  gap_residues = [1u64] of UInt64
  n = 2 if modpn < 2310_u64
  limit = modpn >> 1
  range_n = limit // n
  pc = (range_n - 3)|1
  until pc.gcd(modpn) == 1; pc += 2 end
  while pc < limit
    until pc.gcd(modpn) == 1; pc += 2 end
    gap_residues << pc
    pc += (range_n|1)^1    # pc must be odd, so make range even: lsb = 0
  end
  gap_residues << limit if gap_residues.size < n + 1
  gap_residues
end

def gapcounts(first, last, modpn)
  gaps = Hash(UInt64, UInt64).new(0)
  res = prev_res = first
  inc = res % 6 == 5 ? 2 : 4 
  while res <= last
    if res.gcd(modpn) == 1
      gap = res - prev_res
      gaps[gap] += 1
      prev_res = res
      #print "\r #{(res * 100)/last}%"
    end
    res += inc; inc ^= 0b0110
  end
  #puts
  gaps.delete(0)   # remove init gap size of '0'
  gaps
end

def gaps(p, n = 8)
  primes = [2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47] of UInt64
  i = primes.index(p).not_nil!
  modpn  = primes[0..i].product
  rescnt = primes[0..i].map{ |p| p-1 }.product
  puts "modp#{p} = #{modpn}"
  puts "rescnt = #{rescnt}"
  a_n = Hash(UInt64, UInt64).new(0)  # hash of gap coefficients a_n
  gaps = [] of Hash(UInt64, UInt64)  # array or gap hashes for each segment
  i = 0
  done = Channel(Nil).new()          # process each segment in parallel
  rangeres(modpn, n).each_cons_pair do |n1, n2| 
    spawn do 
      gaps << gapcounts(n1, n2, modpn)
      print "\r#{i += 1} of #{n} segments done"
      done.send(nil)
  end end
  n.times { done.receive }           # wait for all threads to finish
  puts
  gaps.reduce { |h1, h2| a_n = h1.merge(h2) { |key, v1, v2| v1 + v2 } }
  a_n = a_n.to_a.map{ |k, v| (k == 2 || k == 4) ? [k, v*2 | 1] : [k, v*2]}.to_h
  a_n.to_a.sort
end

Here’s the error message.

 25 | if res.gcd(modpn) == 1
             ^--
Error: no overload matches 'Int32#gcd' with type UInt64

But here’s another thing I just noticed:

In gaps I do:

primes = [2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47] of UInt64

and all those values are cast as UInt64s, because all the other values based on them are UInt64s, but not for the single case. This seems like an edge case.

6

So the problem is gcd? We made it so that you always use the same type because using different types could lead to overflow in some cases.

This seems to be unrelated to [1] of UInt64 not working.

In your case, I guess you can do res.to_u64.gcd(modpn) == 1.

7

No, no. I did do it that way to get it working, but the issues is not with gcd, it’s with the [1].

That one value as Int32 caused the problem, but it should have been a UInt64, and doing [1u64] solves it. But why then does initializing primes with multiple values with the same form not create that problem? There is an inconsistency here that is revealed when the math is done with different types.

In the case with one value the compiler didn’t cast it as a UInt64 but in the case with multiple values it did.

8

Sorry I have to repeat this, but can you provide the code that triggers the error in the array?

9

I found the problem @asterite (using 0.35.1).

Sorry, the code I posted was working version, the code below had the problem.

def rangeres(modpn, n)
  gap_residues = [1] of UInt64
  n = 2 if modpn < 2310_u64
  limit = modpn >> 1
  return [1, limit] if n < 2
  range_n = limit // n
  pc = (range_n - 3)|1
  until pc.gcd(modpn) == 1; pc += 2 end
  while pc < limit
    until pc.gcd(modpn) == 1; pc += 2 end
    gap_residues << pc
    pc += (range_n|1)^1    # pc must be odd, so make range even: lsb = 0
  end
  gap_residues << limit if gap_residues.size < n + 1
  gap_residues
end

The problem was not with the array: gap_residues = [1] of UInt64

The “problem” was 3 lines below it: return [1, limit] if n < 2

Even though that line was never going to be executed, the [1, limit] was causing the problem with the compiler; changing it to [1u64, limit] compiles.

I originally “fixed” the problem by doing: res = prev_res = first.to_u64
which forced res|prev_res to be UInt64s in gapscount.

Then realized I didn’t need the problem line, since I eliminated that condition,
and kept gaps = [1u64] of UInt64, but just now saw [1] of UInt64 works as expected.

The original “problem” was my misunderstanding of that early return value being an Int32 causing the compiler to burp about it. So there was no error in the array initialization (which makes me feel good), but the compiler could have been better at pointing out the source of the type Int32|UInit64 conflict, instead of just where the conflict occurred. Well, chalk it up to experience (again).

So what I thought was the problem was incorrect, and we can mark this [SOLVED].

1 Like