Clojure

-> produces known-invalid code when it could be smarter

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

(-> inc (comp read ())

Macroexpands to (nil (comp read promise)), which is known at compile-time to be illegal code.
It seems like it would be harmless, and occasionally useful, to special-case () to expand into wrapping parens around the preceding form, such as:

((comp inc read))

I'll put together a patch for this shortly if nobody tells me it's a terrible idea.

Activity

Hide
Alan Malloy added a comment - - edited

Sorry, that -> form came out wrong. The first line should read: (-> inc (comp read) ())

Show
Alan Malloy added a comment - - edited Sorry, that -> form came out wrong. The first line should read: (-> inc (comp read) ())
Hide
Alan Malloy added a comment - - edited

Here's me testing that my improved versions of -> and ->> work in my use case, and preserve existing behavior for (-> x []) just in case someone was depending on the expansion of this crazy form. Patch will be attached momentarily.

clojure.core> (macroexpand-all '(-> inc (comp read) ()))
((comp inc read))
clojure.core> (macroexpand-all '(->> inc (comp read) ()))
((comp read inc))
clojure.core> (macroexpand-all '(->> read (comp inc) () inc))
(inc ((comp inc read)))
clojure.core> (macroexpand-all '(-> x))
x
clojure.core> (macroexpand-all '(-> x []))
([] x)

Show
Alan Malloy added a comment - - edited Here's me testing that my improved versions of -> and ->> work in my use case, and preserve existing behavior for (-> x []) just in case someone was depending on the expansion of this crazy form. Patch will be attached momentarily. clojure.core> (macroexpand-all '(-> inc (comp read) ())) ((comp inc read)) clojure.core> (macroexpand-all '(->> inc (comp read) ())) ((comp read inc)) clojure.core> (macroexpand-all '(->> read (comp inc) () inc)) (inc ((comp inc read))) clojure.core> (macroexpand-all '(-> x)) x clojure.core> (macroexpand-all '(-> x [])) ([] x)
Hide
Alan Malloy added a comment -

I also factored out some of the behavior common to -> and ->> into a separate function - especially helpful as I was adding still more common behavior. If y'all think it's a good idea I can lift the rest of the common functionality (ie the defns and recursive cases) into a macro that gets called twice, but that seems likely to be less readable and it's not like we're playing golf here.

Show
Alan Malloy added a comment - I also factored out some of the behavior common to -> and ->> into a separate function - especially helpful as I was adding still more common behavior. If y'all think it's a good idea I can lift the rest of the common functionality (ie the defns and recursive cases) into a macro that gets called twice, but that seems likely to be less readable and it's not like we're playing golf here.
Hide
Alan Malloy added a comment -

Having run ant test, I see that defn- isn't actually available at this point in the bootstrapping process; it should read defn ^:private.

Show
Alan Malloy added a comment - Having run ant test, I see that defn- isn't actually available at this point in the bootstrapping process; it should read defn ^:private.
Hide
Stuart Halloway added a comment -

Rich: interested?

Show
Stuart Halloway added a comment - Rich: interested?
Hide
Rich Hickey added a comment -

No

Show
Rich Hickey added a comment - No
Hide
Chas Emerick added a comment -

Closed per request from Alan Malloy.

Show
Chas Emerick added a comment - Closed per request from Alan Malloy.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: