Class name clash between top-level functions and defn'ed ones

Description

Named anonymous fn's are not guaranteed to have unique class names when AOT-compiled.

For example:

When AOT-compiled both functions will emit user$g.class, the latter overwriting the former.

Impact: this affects apps like Cursive, which has been using a patched version of Clojure to get around this issue for quite a while.

Demonstration script: demo1.clj

Patch: 0001-CLJ-1093-v3.patch (already applied, see below)

Approach: Generate unique class names for named fn's the same way as for unnamed anonymous fn's.
The patch contains an additional enhancement to include the name of the local binding in the class name.

Comparison between pre and post patch naming scheme (N denotes unique number):

code

before

after

note

(defn a [])

user$a

user$a

same

(fn [])

user$evalN$fn__N

user$evalN$fn__N

same

(fn a [])

user$evalN$a__N

user$evaN$a__N

same

(let [a (fn [])] a)

user$evalN$a__N

user$evalN$a__N

same

(let [a (fn x [])] a)

user$eval1N$x__N

user$evalN$a_x_N

IMPROVED - contains local binding name

(def a (fn []))

user$a

user$a

same

(def a (fn x []))

user$x

user$a_x_N

FIXED conflict with (defn x [])

(def ^{:foo (fn [])} a)

user$fn__N

user$fn__N

same

(def ^{:foo (fn a [])} a)

user$a

user$a__N

FIXED conflict with (defn a [])

(def a (fn [] (fn [])))

user$a$fn__N

user$a$fn__N

same

(def a (fn [] (fn x [])))

user$a$x__N

user$a$x__N

same

See also: This patch also fixes the issue reported in CLJ-1227.

Screened by: Alex Miller - I am not sure whether the local binding name enhancement is worth doing. It improves debugging of which anonymous class you're talking about but has the downsides of increasing class name (and file name) length.

REOPENED Patch: 0001-CLJ-1093-v3-no-locals-improv.patch
REOPENED UPDATE: The local improvement version of this patch increases class names (and thus .class names) and we've had several reports of exceeding file system limits (~143 chars) - see comments for details. Thus, we should rollback the prior patch (0001-CLJ-1093-v3.patch) and apply the version without this enhancement. The replacement patch (0001-CLJ-1093-v3-no-locals-improv.patch) does not have the same effect on class name length.

Environment

None

Activity

Show:
Nicola Mometto
October 6, 2014, 5:54 PM

Addressing the screening comment by Alex Miller, I've attached an alternative patch "0001-CLJ-1093-v3-no-locals-improv.patch" which is identical to "0001-CLJ-1093-v3.patch" except it doesn't include the local binding name enhancement, so that it can be picked in case Rich decides that that improvement is out of scope for this ticket.

Alex Miller
October 28, 2014, 6:05 PM

I've reopened this issue based on early reports of breakage due to long file names.

Two reports:
https://groups.google.com/d/msg/clojure-dev/hnkJb9_il_M/4e5smM6mVlIJ
https://groups.google.com/d/msg/clojure/hnkJb9_il_M/QOaTdCo5wmkJ

Alex Miller
October 28, 2014, 6:21 PM

Here's an example of a class name that is too long on Ubuntu 12.04 LTS 64bit / Java8 - reported max file size is 143 chars:

https://github.com/ska2342/nested-fn-breaks-clojure-17/blob/master/src/nested_fn_breaks_clojure_17/core.clj

With 1.6.0: (95 chars)
core$this_function_breaks_with_clojure_1_7$my_anonymous_function_18$iter1923$fn24$fn_25

With 1.7.0-alpha3: (144 chars)
core$this_function_breaks_with_clojure_1_7$my_anonymous_function_my_anonymous_function19$iter4951auto__iter2024$fn25$fn_26.class

With the alternate patch here, the name would be: (95 chars)
core$this_function_breaks_with_clojure_1_7$my_anonymous_function_19$iter2024$fn25$fn_26

Nicola Mometto
October 28, 2014, 6:26 PM

patch "0001-CLJ-1330-remove-local-binding-name-enhancement.patch" has the same effect of reverting f149260c14a75367dc9eba91cbe9b78110113566 and applying "0001-CLJ-1093-v3-no-locals-improv.patch" in case this is preferable

Stefan Kamphausen
October 29, 2014, 1:44 PM

The tiny and unusual max file size of 143 is standard in the Ubuntu 12.04 crypto container for the home directory. You can get it for any directory with 'getconf NAME_MAX /path/to/dir'.

My initial problem (other than the file to reproduce on github) was triggered by the fns in a for-expression. Don't know if that makes any difference for you.

Completed

Assignee

Unassigned

Reporter

Nicola Mometto

Labels

Approval

Ok

Patch

Code

Fix versions

Priority

Critical
Configure