The changes from https://clojure.atlassian.net/browse/CLJ-2713 work as expected for positive integers, but they produce different results from 1.11.1 in a variety of other less common cases: 0, negatives, and non-integers. Much of this behavior should be considered undefined, but we have found that some is relied on in production use cases.

Semantics to preserve:

• nthrest docstring promises to return the coll on 0 (not seq of coll)

• drop nthrest and nthnext promise nothing about negative n, but the implementation has long treated them the same as n=0, so we should retain that behavior (alpha1 actually made negative drop "work" on vector sequences)

• drop nthrest and nthnext make no promises about non-integer n, but the implementations have long worked as if by (int (ceil n)) (by relying on loops guarded by pos?). In particular, nthrest is used by partition, and there is some usage in the wild like (partition (/ (count xs) 4) xs) so it’s important that for a fraction < 1, this is treated as (partition 1 xs) (finite result) vs (partition 0 xs) (infinite result).

Alternatives

The main question is whether to handle this in the core functions that call IDrop.drop(), or in the IDrop implementations. Because the drop() call takes an int, the non-integer cases have to be handled in the core functions. These also serve as places to enforce consistent behavior.

IDrop has its own contract which states that if n=0, this is returned. However, that contract returns Sequential, and not all types that implement IDrop are Sequential (like PersistentArrayMap), so something needs to be clarified here. We should either clarify the contract as n must be > 0 (enforced by the core fns), or return a Sequential view of this.

Coercion to conceptually “(int (ceiling n))” is kind of gross, especially if it’s already an int. The best JDK answer for non-integers is probably Math/ceil which takes and returns a double, so that requires conversion via double → Math/ceil → int .

Math/ceil is the best JDK answer here - it returns a double that Java will automatically coerce to int in the call.

Proposal

For consistency, the core functions should:

• Check for pos? before calling IDrop

• Handle coercion to int in a way that matches prior behavior - use int? check and if false, do (Math/ceil) (rely on Java to do the coercion to int).

• Clarify IDrop.drop() to expect only n > 0. All implementations can then assume that and not handle it.

Patch: clj-2772.patch

In addition to above:

• Remove the now unnecessary check for n<0 in PV

• Added a bunch of tests

Screened by: fogus