<< Back to previous view

[CLJS-359] `with-meta` does not work on function objects Created: 25/Aug/12  Updated: 27/Jul/13  Resolved: 21/Nov/12

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Shantanu Kumar Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: bug
Environment:

CLJS Rhino-REPL and during Compilation


Attachments: Text File CLJS-359-v1.patch     Text File CLJS-359-v2.patch    
Patch: Code and Test

 Description   

`with-meta` does not working on function objects in CLJS. Compilation fails with the following error:

Error: No protocol method IWithMeta.-with-meta defined for type function: function { return x; }

I tried it out on the REPL and found the following:

---------- BEGIN: repl-rhino ----------
ClojureScript:cljs.user> (with-meta #(do :foo) {:foo :bar})
"Error evaluating:" (with-meta (fn* [] (do :foo)) {:foo :bar}) :as "cljs.core.with_meta.call(null,(function (){\nreturn \"\\uFDD0'foo\";\n}),cljs.core.ObjMap.fromObject([\"\\uFDD0'foo\"],{\"\\uFDD0'foo\":\"\\uFDD0'bar\"}));\n"
org.mozilla.javascript.JavaScriptException: Error: No protocol method IWithMeta.-with-meta defined for type function:
function () { return "\ufdd0'foo"; }
(cljs/core.cljs#222)
at cljs/core.cljs:222 (anonymous)
at cljs/core.cljs:214 (_with_meta)
at cljs/core.cljs:806 (with_meta)
at <cljs repl>:2 (anonymous)
at <cljs repl>:2

nil
---------- END: repl-rhino ----------

Reference: https://groups.google.com/forum/?fromgroups=#!topic/clojure/pRO-5IlilNM



 Comments   
Comment by David Nolen [ 29/Aug/12 9:52 AM ]

We could create a fn wrapper type of some kind that implements all the supported arities of IFn (this means the compiler needs to guarantee that args past 20 are collected into an array-seq) that include the extra meta fields. Seems doable.

Comment by Paul deGrandis [ 28/Sep/12 3:21 PM ]

I have created that exact function wrapper - it's in Shoreleave. We can move it into CLJS proper if you like.

https://github.com/shoreleave/shoreleave-core/blob/master/src/shoreleave/efunction.cljs

Comment by David Nolen [ 28/Sep/12 3:24 PM ]

That's nice but would prefer to have as little overhead as possible.

Comment by Paul deGrandis [ 28/Sep/12 3:27 PM ]

ala unrolling the invoke call?

More than happy to start pulling together a patch.

Comment by David Nolen [ 28/Sep/12 3:34 PM ]

Yes there's actually an existing ticket for that. Thanks. We also need compiler support for function invocations when there's more then 20 arguments - though that's a different existing ticket.

Comment by Paul deGrandis [ 22/Oct/12 7:50 AM ]

I was just waiting on CLJS-365 to be completed. Did you think of a better approach than the deftype-wrapper? More than happy to push on this.

Comment by David Nolen [ 23/Oct/12 6:36 PM ]

I thought about this some more - this patch is really about adding metadata to functions at runtime - static metadata on defs are stored in cljs.analyzer/namespaces. Given that it's runtime I think your simple patch is actually OK, we can't generally optimize functions like that anyway since we won't have that kind of information available.

Comment by Brandon Bloom [ 21/Nov/12 3:29 PM ]

Patch adds a Function type and extends js/function to support metadata. There is now an Fn marker protocol.

Patch also includes an additional commit to fix the behavior of the this variable for IFn definitions.

Not tested on JavaScriptCore.

Comment by Brandon Bloom [ 21/Nov/12 4:16 PM ]

v2 of patch improves speed of fn? and utilizes reify to avoid creating an explicit Function type

Comment by David Nolen [ 21/Nov/12 4:21 PM ]

fixed, http://github.com/clojure/clojurescript/commit/6ce3b1cef3824fd36e75402f5a8ed5053252b15e

Generated at Wed Jul 30 00:13:01 CDT 2014 using JIRA 4.4#649-r158309.