ClojureScript

Outward function type hint propagation

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

New feature: Detect the type of a function and arrange so that it is available for outward type hinting.

For reference, a small amount of discussion from Jan 2016 in the #cljs-dev Slack:

mfikes: Curious if "outward" ^boolean propagation has been debated / rejected. Example: If simple analysis detects that ^boolean can be applied as in for functions like (defn f [x] (not x)). Perhaps automatically propagating type hints in this direction would be unwelcome by users?

dnolen: @mfikes: we have already have outward propagation for numerics which broke because of changes for code motion

dnolen: absolutely nothing against that

Activity

Hide
Mike Fikes added a comment -

The attached patch will handle cases like

(defn foo? [x] (or (string? x) (number? x)))

and

(def baz (fn ([x] 1) ([x y] 2)))

but it doesn't attempt to address the case of multi-arity defn s, as these are implemented by dispatching to multiple single-arity functions that could have different return types.

It adds a :inferred-ret-tag to the AST only because the arguments to or on line 18 in the patch are not reversed (only to allow explicit hints to override inferred hints, but perhaps that is overcomplicating things and the inferred tag could be dropped directly into :ret-tag.)

Bronsa helpfully pointed out in Slack that outward type hint propagation is avoided in Clojure because it breaks things like:

(defn foo [] 1)
(defn bar [] (String/valueOf (foo)))
(defn foo [] "foo")
(bar)

for which an analogy in ClojureScript would be:

(defn foo? [] true)
(defn bar [] (if (foo?) :t :f))
(defn foo? [] "")
(bar)

where the patch causes it to yield :f.

This could be viewed as a REPL-only concern (perhaps addressed by another compiler flag along the lines of :static-fns).

But, arguably the problem already exists today, with type hinting in code like the following affecting the compilation of bar:

(def foo? true)
(defn bar [] (if foo? :t :f))
(def foo? "")
(bar)
Show
Mike Fikes added a comment - The attached patch will handle cases like
(defn foo? [x] (or (string? x) (number? x)))
and
(def baz (fn ([x] 1) ([x y] 2)))
but it doesn't attempt to address the case of multi-arity defn s, as these are implemented by dispatching to multiple single-arity functions that could have different return types. It adds a :inferred-ret-tag to the AST only because the arguments to or on line 18 in the patch are not reversed (only to allow explicit hints to override inferred hints, but perhaps that is overcomplicating things and the inferred tag could be dropped directly into :ret-tag.) Bronsa helpfully pointed out in Slack that outward type hint propagation is avoided in Clojure because it breaks things like:
(defn foo [] 1)
(defn bar [] (String/valueOf (foo)))
(defn foo [] "foo")
(bar)
for which an analogy in ClojureScript would be:
(defn foo? [] true)
(defn bar [] (if (foo?) :t :f))
(defn foo? [] "")
(bar)
where the patch causes it to yield :f. This could be viewed as a REPL-only concern (perhaps addressed by another compiler flag along the lines of :static-fns). But, arguably the problem already exists today, with type hinting in code like the following affecting the compilation of bar:
(def foo? true)
(defn bar [] (if foo? :t :f))
(def foo? "")
(bar)
Hide
David Nolen added a comment -

Thanks will think about it

Show
David Nolen added a comment - Thanks will think about it
Hide
Mike Fikes added a comment -

I've been soaking this for a couple of months (with local builds of Planck), and haven't seen anything break, FWIW. So, I think it is at least a safe change. Assigning to David for further consideration.

Show
Mike Fikes added a comment - I've been soaking this for a couple of months (with local builds of Planck), and haven't seen anything break, FWIW. So, I think it is at least a safe change. Assigning to David for further consideration.

People

Vote (1)
Watch (4)

Dates

  • Created:
    Updated: