TermTest fails for Range with Elixir v1.12

[evnu]

On master (ec1cd5b) with Elixir v1.12.2:

  1) test term_debug (RustlerTest.TermTest)
     test/term_test.exs:4
     Assertion with == failed
     code:  assert RustlerTest.term_debug(0..1000) == "\#{'__struct__'=>'Elixir.Range',first=>0,last=>1000}"
     left:  "\#{'__struct__'=>'Elixir.Range',first=>0,last=>1000,step=>1}"
     right: "\#{'__struct__'=>'Elixir.Range',first=>0,last=>1000}"
     stacktrace:
       test/term_test.exs:10: (test)

The step field was added with v1.12 to Elixir's Range. This is unfortunate, as Range no longer maps to Rust's std::ops::RangeInclusive.

EDIT: The test above is simple to fix (allow both the old and new form to match), but it's not clear how to handle Decoder for RangeInclusive<T>.

[filmor]

Decoding this to always include step should be fine, shouldn't it? I checked in Elixir 1.11, creating the struct manually as a map including the step entry works and it's recognised as a Range.

[evnu]

The problem is that the correct step is lost when decoding Range.new(from, to, step), as RangeInclusive doesn't store a step. So if a user uses a Range.new(0, 10, 2) and pushes that into a round-trip function, the result of the round-trip would be Range.new(0, 10, 1). We could wrap RangeInclusive in a new type and store the step separately, but this new type would not work the same way RangeInclusive does I guess.

[evnu]

I currently see these options here:

a. Require that step is either nil (pre-Elixir v1.12), or that it equals 1
b. Remove the RangeInclusive Encoder/Decoder completely and leave it up to users to implement it
c. Add a wrapping type RangeInclusiveWithStep, which also holds the step.

Opinions?

[philss]

I like the option c, but if it's too complicated to add I would recommend option a for now.

[evnu]

#399 implements option a. We don't have an encoder for RangeInclusive anyways, so requiring that the step is 1 (if present at all) seems to be the right way to handle this.