CLJ-2141

New qualified-* predicates can return true, nil, and false

Description

As of Clojure 1.9-alpha15, the new qualifed-keyword?, qualified-symbol?, and qualified-ident? functions can return true, nil, or false depending on the input passed to them, e.g.:

1 2 3 4 5 6 => (qualified-keyword? ::abc) true => (qualified-keyword? :abc) nil => (qualified-keyword? 'abc) false

Returning nil rather than false for unqualified keywords has the following issues:

  • Many sources of Clojure documentation note that a trailing '?' in a Clojure function generally implies a true/false return so this violates a common expectation.

  • Returning true/false/nil complicates the ability of ClojureScript to optimize boolean returns. Theoretically, this might affect Clojure JIT optimization as well, turning a call taking the return value into polymorphic rather than bimorphic.

  • Returning true/false/nil can yield confusing results for functions that partition by value, like group-by

The prior implementation during development used `boolean` to coerce the return value to a null and that was removed for performance reasons (would require a call through another var).

Alternatives:

Perf benchmark (w/ criterium bench, Java 1.8, AOT, direct-linked)
Example: (bench (dotimes [_ 100] (qualified-keyword? :a)))

Impl

:a

:a/b

note

(and (keyword? x) (namespace x) true)

96.749127 ns

96.412551 ns

current impl (returns nil for :a)

(and (keyword? x) (some? (namespace x)))

99.014677 ns

96.149567 ns

uses existing preds

(and (keyword? x) (not (nil? (namespace x))))

97.512916 ns

95.008061 ns

nil? inlines

(if (and (keyword? x) (namespace x)) true false)

98.617369 ns

97.866673 ns

if-based

(if (keyword? x) (if (namespace x) true false) false)

99.000727 ns

99.474754 ns

Note that these vary by less than 1 ns per invocation, so perf is not significantly different.

Approach: After discussion with Rich, have decided to use a boolean wrapper around the check: (boolean (and (keyword? x) (namespace x))).
Also, needed to move boolean implementation higher to colocate the qualified fns near the other fns.

Patch: clj-2141-3.patch

Environment

None

Status

Assignee

Unassigned

Reporter

Daniel Compton

Labels

Approval

Ok

Patch

Code

Fix versions

Affects versions

Release 1.9

Priority

Major