Update and uniformise lists specs #8736
Conversation
Uniformise specs to adhere to the proposed [EEP 71](erlang/eep#63). Many specs use `when` constraints and `::` inconsistenly, making it hard for type-checkers to reason about. For example, ``` -spec reverse(List1) -> List2 when List1 :: [T], List2 :: [T], T :: term(). ``` in this spec, `List1` and `List2` are arguably defined as aliases for `[T]`, while `T` is defined to be "a subtype of" `term()`. Removing the last constraint makes it clearer that the returned list has the same type as the input list: ``` -spec reverse(List1) -> List2 when List1 :: [T], List2 :: [T]. ``` This PR unifies all specs of the lists module under this principle, while minimising the amount of changes to specs: it keeps the spirit of using aliases and `when` constraints instead of inlining types.
|
|
CT Test Results 2 files 95 suites 56m 48s ⏱️ Results for commit 50776e6. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
- Remove recursive constraints in DeepLists to adhere to EEP 71 - Rename variables in docs according to specs
lib/stdlib/src/lists.erl
Outdated
| @@ -102,8 +102,7 @@ equal to `Key`. Returns `Tuple` if such a tuple is found, otherwise `false`. | |||
| -spec keyfind(Key, N, TupleList) -> Tuple | false when | |||
| Key :: term(), | |||
| N :: pos_integer(), | |||
| TupleList :: [Tuple], | |||
| Tuple :: tuple(). | |||
| TupleList :: [Tuple]. | |||
There was a problem hiding this comment.
Doesn't Kiko's comment apply to this too? (And throughout).
If you're not going to demand tuples (technically correct, but really confusing), then don't call it a TupleList
There was a problem hiding this comment.
Also, what is this really saying... "the result Tuple must have been part of the original list"? Kind of odd (borderline unhelpful?) losing track of that the result will always be a tuple.
There was a problem hiding this comment.
I've updated the aliases and the corresponding docs to remove the mention of TupleList, since it is not actually required to be all tuples.
Yes this states that the result is part of the original list, which IMO conveys more information than just saying it is a tuple().
If your input is [{atom(), atom()}] then your output will be {atom(), atom()}, which is more precise than just tuple().
- Clarify in docs and aliases functions for which not all elements have to be tuples, e.g., `lists:keyfind`. Aliases for lists are renamed from `TupleList` to `List`, and elements from `Tuple` to `Elem`. - Revert changes to specs for functions that actually need a list of tuples, such as `lists:keymerge` and `lists:keysort` variants. Enforce `[tuple()]` arguments for these functions.
|
In order to make sure that the implementation and the specs match I think that for any spec where you change the argument types you should write new testcases that verify that this is true. If such a testcase already exists then add a comment here on github pointing to the test that verifies it. |
| -spec keyfind(Key, N, TupleList) -> Tuple | false when | ||
| -spec keyfind(Key, N, List) -> Elem | false when | ||
| Key :: term(), | ||
| N :: pos_integer(), | ||
| TupleList :: [Tuple], | ||
| Tuple :: tuple(). | ||
| List :: [Elem]. |
There was a problem hiding this comment.
In the current state of the type system, we lack bounded quantification. If we were to accept this type as written, our users will lose type information because not all type systems can infer that Elem must be a subtype of tuple(), more specifically, of the tuple passed as argument in the list.
I understand that is the more precise one can get, but it also means losing type information.
If we add bounded quantification, we could say:
-spec keyfind(Key, N, List) -> Elem | false when
Key :: term(),
N :: pos_integer(),
List :: [Elem],
Elem <: tuple().In this case, some type systems may choose Elem = tuple() and move on, and strong type systems may figure out the exact tuple value expected. For now, I do not think we can go with this level of generic programming
Uniformise specs to adhere to the proposed EEP 71.
Many specs use
whenconstraints and::inconsistenly, making it hard for type-checkers to reason about.For example,
in this spec,
List1andList2are arguably defined as aliases for[T], whileTis defined to be "a subtype of"term().Removing the last constraint makes it clearer that the returned list has the same type as the input list:
This PR unifies all specs of the lists module under this principle, while minimising the amount of changes to specs: it keeps the spirit of using aliases and
whenconstraints instead of inlining types.