some-fn has different short-circuiting when using 3 predicates

Description

Per https://ask.clojure.org/index.php/10366/some-has-different-short-circuiting-when-using-predicates

All branches of the unrolled some-fn check all arg values with p1, then all arg values with p2, etc EXCEPT with 3 predicates and 2 or 3 args where all args are checked for p1, then all args for p2, etc.

with 2 predicates:

((some-fn #{42} #{1}) 1 42) => 42

with 3 predicates:

((some-fn #{42} #{1} #{1}) 1 42) => 1

with 4 or more predicates:

((some-fn #{42} #{1} #{1} #{1}) 1 42) => 42

For the 3-predicate case, the checks being done are:

(#{42} 1) ;; p1, arg 1 (#{1} 1) ;; p2, arg 1 ;; SHORT-CIRCUIT and return ---- would continue: (#{1} 1) ;; p3, arg 1 (#{42} 42) ;; p1, arg 2 (#{1} 42) ;; p2, arg 2 (#{1} 42) ;; p3, arg 2

But should be all p1, then all p2, etc to match all other unrollings and the generic case:

(#{42} 1) ;; p1, arg 1 (#{42} 42) ;; p1, arg 2 ;; SHORT-CIRCUIT and return ---- would continue: (#{1} 1) ;; p2, arg 1 (#{1} 42) ;; p2, arg 2 (#{1} 1) ;; p3, arg 1 (#{1} 42) ;; p3, arg 2

Additionally, every-pred has the same unrolling and the same bad ordering. In that case, we are dealing with boolean values instead of logical true values and and rather than or so the actual result is correct either way.

Proposed:

  • Fix the predicate order check in some-fn for the 3-predicate 2- and 3-arg cases to match all other cases.

  • Fix the predicate order check in every-pred for these cases just for consistency (even though this should have no observable effect).

  • Add tests for the cases that were bad.

Patch:

clj-2649-2.patch

Environment

None

Attachments

1
  • 20 Aug 2021, 09:26 PM

Activity

Show:

Alex MillerSeptember 14, 2021 at 3:08 PM

Applied for 1.11.0-alpha2

Michael FogusAugust 23, 2021 at 6:09 PM

Screened. It should be noted that release notes should say that if users of some-fn relay on that specific behavior (i.e. depend on it returning specific values) then they should stop doing that.

Fixed

Details

Assignee

Reporter

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created July 29, 2021 at 12:41 AM
Updated September 14, 2021 at 3:08 PM
Resolved September 14, 2021 at 3:08 PM

Flag notifications