Fix key type for empty NamedTuple be Symbol

[caspiano]

This PR changes the semantics of to_a and to_h on an empty NamedTuple and includes documentation of said behaviour.

After this PR, the following behaviour is expected...

t = NamedTuple.new
typeof(t.to_h) # => Hash(Symbol, NoReturn)
typeof(t.to_a) # => Array(Tuple(Symbol, NoReturn))

Resolves #9923
Resolves #10939

[straight-shoota]

This is technically breaking, but I'd consider it a bug fix. And it really shouldn't have a noteworthy negative impact.

[straight-shoota]

Btw. is there any reason why #to_a fills the collection manually whereas #to_h constructs a literal?

[caspiano]

No idea, @straight-shoota, I just made a minor change. I can change it to construct a literal?

Edit: Done ✅

[beta-ziliani]

Thanks @caspiano !

[straight-shoota]

I was actually just asking about literal vs. construcing in general, not specifically asking to replace one by the other. 🙈 I'd like to explicitly consider which one's actually preferable.

[caspiano]

@straight-shoota I ran some benchmarks, looks like the literal approach is faster.

require "benchmark"

struct NamedTuple
  def literal_to_a
    {% if T.size == 0 %}
      [] of {Symbol, NoReturn}
    {% else %}
      [
        {% for key in T %}
          { {{key.symbolize}}, self[{{key.symbolize}}] },
        {% end %}
      ]
    {% end %}
  end

  def dynamic_to_h
    hash = Hash(typeof(first_key_internal), typeof(first_value_internal)).new(size)
    each do |key, value|
      hash[key] = value.as(typeof(first_value_internal))
    end
    hash
  end
end

test = {
  a: 1,
  b: "1",
  c: Time.unix(1),
  d: [1]
}

puts "NamedTuple.to_a"
Benchmark.ips do |x|
  x.report("literal") { test.literal_to_a }
  x.report("each") { test.to_a }
end

puts "NamedTuple.to_h"
Benchmark.ips do |x|
  x.report("literal") { test.to_h }
  x.report("each") { test.dynamic_to_h }
end

$ crystal run --release bench.cr
NamedTuple.to_a
literal  13.48M ( 74.19ns) (± 7.33%)  208B/op        fastest
   each  11.68M ( 85.61ns) (± 7.13%)  208B/op   1.15× slower
NamedTuple.to_h
literal   8.48M (117.94ns) (± 7.31%)  240B/op        fastest
   each   7.38M (135.47ns) (± 8.14%)  256B/op   1.15× slower

[HertzDevil]

first_key_internal should be unused after this PR now that #10950 is also merged.