great explanation Eric, I was just missing that, thus my concern. I will submit a PR then changing Enum.index/0 to be `integer`, if nobody opposes
On Fri, 8 Jul 2016 04:44:57 +0200 Eric Meadows-Jönsson <[email protected]> wrote: > The type is not documented with any explanation of its intended usage > so if people use it they should only use it in the context of Enum > functions using the type. If they use it any other way I would say > they are using it incorrectly and we can't cover incorrect usage > against backwards incompatibility. > > Therefor changing the type is backwards incompatible as long as we > don't break any APIs in the Enum module using it. > > On Fri, Jul 8, 2016 at 1:33 AM, eksperimental > <[email protected]> wrote: > > > sorry for the latish reply. > > > > I mean it will be backward incompatible because other people may > > rely on this type, to either specify other types in the modules, or > > functions. > > and since we will be changing it, regarless of being broader of > > narrower, that could cause trouble. > > > > Enum.index/0 is only used in Enum.find_index/2, but if changed to > > `integer` it could be used in all these functions > > > > $ ag "@spec\s+[^\n]+\binteger" lib/elixir/lib/enum.ex > > 313: @spec at(t, integer, default) :: element | default > > 564: @spec drop(t, integer) :: list > > 718: @spec fetch(t, integer) :: {:ok, element} | :error > > 769: @spec fetch!(t, integer) :: element | no_return > > 1909: @spec slice(t, integer, non_neg_integer) :: list > > 2073: @spec split(t, integer) :: {list, list} > > 2175: @spec take(t, integer) :: list > > 2465: @spec with_index(t) :: [{element, integer}] > > 2466: @spec with_index(t, integer) :: [{element, integer}] > > > > On Thu, 7 Jul 2016 07:51:29 -0700 (PDT) > > Wiebe-Marten Wijnja <[email protected]> wrote: > > > > > I agree with Eric Meadows-Jönnson. Making the typing more general > > > shouldn't introduce incompatible changes. > > > > > > Indices are only used in a few places inside Enum. Access talks > > > about 'keys' which can be anyting. An Index can only be an > > > integer, but it is possible to pass a negative integer to > > > functions like Enum.fetch/2, which will then be 'counted from the > > > end'. > > > > > > I think that either we want to deprecate Enum.index/0 because it > > > does not add a lot more information over just using `integer` or > > > `non_neg_integer` as type. In fact, because of above-stated > > > ambiguity, one might argue that using `integer` or > > > `non_neg_integer` is more explicit. > > > > > > Or, we want to make it more general, so it can be used in more > > > places such as Enum.fetch. > > > > > > In fact, it is somewhat strange that functions like > > > `Enum.with_index` do not use `Enum.index` in their return typing. > > > This might be a hint that it is superfluous and could be > > > deprecated/at some point removed. > > > > > > On Tuesday, July 5, 2016 at 4:30:52 AM UTC+2, Eric Meadows-Jönsson > > > wrote: > > > > > > > > I'm not sure what backwards compatibility means when it comes to > > > > typespecs or if we have any rules for it in Elixir. All code > > > > still works the same way when changes are made to typespecs so > > > > what is the potential incompatibility? External tools (like > > > > dialyzer) may be affected but we don't control those and they > > > > can chose to interpret typespecs however they want. > > > > > > > > I don't think making a typespec less restrictive is a backwards > > > > incompatible change if no APIs change. It also depends on if the > > > > type is used as arguments to functions or as return values. > > > > Accepting more values in arguments is backwards compatible, but > > > > returning more types of values is not. > > > > > > > > The solution might be to instead deprecate Enum.index/0 if > > > > there is no common type that makes sense for all indexes Elixir > > > > use. > > > > > > > > On Mon, Jul 4, 2016 at 6:37 PM, eksperimental > > > > <[email protected] <javascript:>> wrote: > > > > > > > >> I just noticed that the Enum.index/0 type is set to > > > >> `non_neg_integer`, the thing is that indexes are `integer`. > > > >> > > > >> as of now, it is only used one in the whole elixir code base, > > > >> in Enum.find_index/2 > > > >> > > > >> The rest of the times, when we need to represent an index, we > > > >> need to use `integer`, > > > >> I know this change is backward incompatible (and probably have > > > >> to wait until Elixir v2.0), as other libraries/apps may rely on > > > >> Enum.index/0, > > > >> > > > >> but it would make a lot of sense to change it to `integer` and > > > >> update Enum.find_index/2 to use `non_neg_integer` instead of > > > >> index/0. > > > >> > > > >> -- > > > >> You received this message because you are subscribed to the > > > >> Google Groups "elixir-lang-core" group. > > > >> To unsubscribe from this group and stop receiving emails from > > > >> it, send an email to [email protected] > > > >> <javascript:>. To view this discussion on the web visit > > > >> > > https://groups.google.com/d/msgid/elixir-lang-core/20160704233754.6acc2ca5.eksperimental%40autistici.org > > > > > >> . > > > >> For more options, visit https://groups.google.com/d/optout. > > > >> > > > > > > > > > > > > > > > > -- > > > > Eric Meadows-Jönsson > > > > > > > > > > > -- > > You received this message because you are subscribed to the Google > > Groups "elixir-lang-core" group. > > To unsubscribe from this group and stop receiving emails from it, > > send an email to [email protected]. > > To view this discussion on the web visit > > https://groups.google.com/d/msgid/elixir-lang-core/20160708063358.78710f26.eksperimental%40autistici.org > > . > > For more options, visit https://groups.google.com/d/optout. > > > > > -- You received this message because you are subscribed to the Google Groups "elixir-lang-core" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/20160708235626.630d8857.eksperimental%40autistici.org. For more options, visit https://groups.google.com/d/optout.
