CLJ-1227: Simply closing as declined is no solution

394 views
Skip to first unread message

Tassilo Horn

unread,
Jan 22, 2014, 3:33:00 AM1/22/14
to cloju...@googlegroups.com
Hi all,

I'm deeply concerned about CLJ-1227 simply being closed with resolution
declined.

Here's a short summary of the bug: When you have a definline (such as
clojure.core/ints, longs, doubles,...) and you use it as HOF in
AOT-compiled code, its result is the expansion rather than the value of
applying the definline-defined function.

Example:

;; ints is a standard clojure.core definline
(defn -main [& args]
(let [ary (make-array Integer/TYPE 5)]
(println (apply ints [ary]))
(println (ints ary))))

Both printlns should of course evaluate to the very same result. In the
first call, the function ints should be applied, in the second call the
(ints array) should be expanded to (. clojure.lang.Numbers ints ary) by
the compiler to save one function call.

However, the output is:

(. clojure.lang.Numbers clojure.core/ints #<int[] [I@3b9690f6>)
#<int[] [I@3b9690f6>

That's the bug: if a definline is AOT-compiled, then when it's applied
as HOF, it returns the expanded form rather than the value of the
function. And clojure itself is always AOT-compiled.

If you define a definline foo in your own code, then (apply foo ...)
works correctly if it's not AOT-compiled, and if it is AOT-compiled, you
get the same issue.

So what does this mean? It means, you must not use definlines as HOFs
if they are defined by clojure itself (because that's always
AOT-compiled), or if they are defined elsewhere and AOT-compiled.

And how can you know that you're using a definline? Exactly, you can't!
They expand to normal defns with :inline metadata. Their docs are
exactly the same as if they were "normal" functions without :inline
metadata.

user> (doc ints)
-------------------------
clojure.core/ints
([xs])
Casts to int[]
nil


So to wrap up: CLJ-1227 means that clojure.core/doubles, ints, longs,
etc. are always broken when used as HOF, and if you're AOT-compiling
your application (e.g., to make it usable from Java), then any definline
defined by any of your (transitive) dependencies will malfunction.
Since you have no way to check for definlines in your deps (other than
looking up the sources), this basically boils down to: AOT-compiled code
is likely to be broken.

I've debugged the issue and found out that normal defns with :inline
metadata (which is essentially a macro that's used by the compiler) work
fine, no matter if AOT or not. definline expands to a defn whose
metadata is altered after the defn to assoc an :inline macro. So
definline is just a convenience macro for defining a defn with :inline
metadata. The difference is just that with the latter, the :inline
metadata is added after the defn form itself. This was the difference,
and I've written a patch to CLJ-1227 which lets definline expand to a
defn form where the :inline metadata is added immediately. This fixes
the bug: with that patch, definlines can be applied as HOF in both AOT-
as well as non-AOT-compiled code.

I confess that this is more a symptomatic cure. I can't say why

(defn foo
{:inline (fn [x] ...)}
[x]
...)

and

(do
(defn foo [x] ...)
(alter-meta! #'foo assoc :inline (fn [x] ...)))

are equivalent when not being AOT-compiled, but different when being
AOT-compiled.

Now the issue has been closed as declined. Alex' comment is:

,----[ CLJ-1227: Closing comment by Alex Miller ]
| definline should be considered to be like defmacro in that it is not a
| function and cannot be used as a HOF. Additionally, definline is
| considered to be an experimental feature and Rich would like to
| discourage its use as the hope is to remove it in the future. The
| desired replacement is something like common lisp compiler macros that
| could allow the compiler to detect special situations and optimize the
| result but leave behind a function invocation for the case where no
| special behavior is available.
|
| I am declining the ticket based on the info above and per Rich's
| request.
`----

First of all, definline does create a function (and its doc states
that). As said, it expands to a defn which gets attached :inline
metadata later. Therefore, they can be used as HOF, but then they are
not inlined.

Secondly, yes, it's marked experimental. But with all "experimental" or
"alpha" stuff (transients, watches, promises, records, types, reducers
anyone?), it's still in use.

So we have a very severe bug: You cannot trust your application/lib in
AOT-scenarios, since any of your dependencies could have a definline (or
introduce one with any new version) which will malfunction then.

I can only see two valid options how to progress:

1. Apply my patch attached to CLJ-1227 (or another patch fixing the
issue).
2. Delete definline altogether.

I'm fine with both options, but simply doing nothing and releasing
Clojure 1.6 with such a major bug when there's already a fix seems like
complete insanity to me.

If option 2 is not feasible in such a short timeframe, I'm happy to
rework my patch so that definline's docstring clearly indicates that
it's going to be removed in the future (say, 1.7) and also issues a big
fat warning at compile-time.

Bye,
Tassilo

Colin Fleming

unread,
Jan 22, 2014, 5:28:29 AM1/22/14
to cloju...@googlegroups.com
This is originally my bug (I develop Cursive which is totally AOT compiled) and it caused me significant pain until I worked out what was going wrong. I hadn't considered all the ramifications as Tassilo has done (its transitive nature and the fact that Clojure itself uses this feature is indeed very bad), and I agree with him - if his patch works it seems unwise not to apply it just because we'll probably remove definline in the future. If we're going to do that, then in my opinion we should definitely mark it as deprecated rather than experimental, and I agree that a compile-time warning would be a great idea.

Cheers,
Colin



--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.
To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/groups/opt_out.

Tassilo Horn

unread,
Jan 22, 2014, 6:01:44 AM1/22/14
to cloju...@googlegroups.com
Tassilo Horn <ts...@gnu.org> writes:

> So to wrap up: CLJ-1227 means that clojure.core/doubles, ints, longs,
> etc. are always broken when used as HOF, and if you're AOT-compiling
> your application (e.g., to make it usable from Java), then any
> definline defined by any of your (transitive) dependencies will
> malfunction. Since you have no way to check for definlines in your
> deps (other than looking up the sources), this basically boils down
> to: AOT-compiled code is likely to be broken.

Just as a short addendum: here are a few relatively popular Clojure libs
that actually use definline and thus might malfunction with
AOT-compilation.

- Prismatic/hiphip
- aboekhoff/congomongo
- clojure/core.rrb-vector
- clojurewerkz/mailer
- dakrone/cheshire
- dakrone/clj-http
- dakrone/clj-http-async
- javagems/compojure
- jimpil/Clondie24
- metalog/hbase-clj
- michaelklishin/monger
- michaelklishin/neocons
- michalmarczyk/flexvec
- mortuosplango/frankentone
- runa-dev/kits
- shenfeng/rssminer
- stuartsierra/clojure-time-trials
- zakwilson/imagesieve
- ztellman/byte-streams
- ztellman/immutable-bitset
- ztellman/narrator
- ztellman/potemkin
- ztellman/vertigo

This list is far from complete. I've just used the code search on
github and picked out the projects where the project name is popuar
(measure: "I know it although haven't used it so far"), or the author is
prominent (measure: "I know him from the clojure lists or from JIRA").

So if your AOT-compiled project depends on one of those (directly or as
a dependency of a dependency), cross your fingers that the bug won't hit
you!

Bye,
Tassilo

Alex Miller

unread,
Jan 22, 2014, 9:54:12 AM1/22/14
to cloju...@googlegroups.com
Hey Tassilo,

The definline docstring says:

clojure.core/definline
([name & decl])
Macro
  Experimental - like defmacro, except defines a named function whose
  body is the expansion, calls to which may be expanded inline as if
  it were a macro. Cannot be used with variadic (&) args.

The two things to note here are:

1) "Experimental" means "this is an experiment - use at your peril". It is intentionally not "Alpha". Alpha means "this is a feature but we're not 100% sure yet that the exposed API is right so it might change". The bulk of the "alpha" designations in existing Clojure were removed in 1.6 (with the exception of reducers and one or two other things). afaik, the only things marked Experimental in Clojure are definline and one function in clojure.test.

2) "like defmacro" is intended to convey that a definline should not be expected to work as a HOF. Admittedly, that is subtle. :) 

The title of CLJ-1227 ticket is "Definline functions do not work as higher-order functions when AOT compiled". Per #2, don't do that and per #1, at your peril. While I appreciate that there is a patch on the ticket, the decision in this case was to not spend time on this issue.

Rich sees compiler macros as a strictly better solution than definline and would prefer to move in that direction for this kind of functionality. I appreciate the list of projects using definline and I think that further underscores that simply removing it is not an option - there are people using it (without using it as a HOF) successfully. 

I don't personally see that changing the word "Experimental" to "Deprecated" in the doc string would change anything. To me those both mean, you probably shouldn't use this unless you're aware of the ramifications.

I have created a skeleton design page for inlined code here: http://dev.clojure.org/display/design/Inlined+code

Alex

Ambrose Bonnaire-Sergeant

unread,
Jan 22, 2014, 9:58:04 AM1/22/14
to cloju...@googlegroups.com
Would a patch be accepted that eliminates all usages of definline in clojure.core?

Thanks,
Ambrose


--

Marshall Bockrath-Vandegrift

unread,
Jan 22, 2014, 10:18:40 AM1/22/14
to cloju...@googlegroups.com
On Wed, Jan 22, 2014 at 9:54 AM, Alex Miller <al...@puredanger.com> wrote:

> 2) "like defmacro" is intended to convey that a definline should not be
> expected to work as a HOF. Admittedly, that is subtle. :)

I disagree that this is a reasonable interpretation. I've always
taken it to mean "the inline expansion is like defmacro." If the
value held by the resulting var can't then be passed as a regular
function, then `definline` is no different from `defmacro`. I think
it's fairly obvious that the intent of the feature is create objects
which have function values, but expand inline like macros when in the
function call position. Which is exactly how it works, except under
AOT.

-Marshall

Devin Walters

unread,
Jan 22, 2014, 10:49:34 AM1/22/14
to cloju...@googlegroups.com
Along the same lines as Ambrose' question: Would a docstring patch be accepted to make it clearer to people not to treat it as a HOF?

Devin

Alex Miller

unread,
Jan 22, 2014, 10:57:41 AM1/22/14
to cloju...@googlegroups.com, abonnair...@gmail.com
There's not really any point to this. There are only a handful of functions declared as definline in Clojure and they are all casts to prim array types. Those functions will have no effect if used as a HOF, so there is no reason to use them that way. 

Stuart Halloway

unread,
Jan 22, 2014, 11:08:39 AM1/22/14
to cloju...@googlegroups.com
Devin: a patch that made the documentation string more clear would be ok.

Tassilo:  As already stated, the defect here is poor communication about limitations.  Certainly we should communicate that what you want does not work, and will not be made to work.  I am mildly amused that you acknowledge not completely understanding the issue, but characterize the actions of those who do understand it as "insanity". 

Stu

Alex Miller

unread,
Jan 22, 2014, 11:14:18 AM1/22/14
to cloju...@googlegroups.com
As far as I can tell, this has impacted two people over several years of its existence. 

We (mostly me) have now spent a couple hours looking at the ticket, the patch, talking about, and answering the questions for this issue. Some of that was valuable in understanding the problem and a potential path for a better solution. The judgement at this point is that based on impact, future plans, and direction from Rich, we do not intend to spend further time on it now. 

Alex

Michał Marczyk

unread,
Jan 22, 2014, 11:19:27 AM1/22/14
to cloju...@googlegroups.com
clojure.core usages of definline are all casts. Happily there's not
much point to using these in a higher-order fashion, but if they
simply cannot be so used, why not define them using defmacro instead?
I suppose this is a +1 to Ambrose's proposal.

Also, if the existing issues are never going to be fixed, why not
document it as deprecated and unusable with AOT compilation to save
people some nasty surprises?

Cheers,
Michał


PS. core.rrb-vector's presence on the list is due to a lone definline
form which is currently dead code -- a relic of some early experiments
that I abandoned for several reasons; running into weird issues was
one of them.


On 22 January 2014 16:18, Marshall Bockrath-Vandegrift
> --
> You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

Michał Marczyk

unread,
Jan 22, 2014, 11:21:49 AM1/22/14
to cloju...@googlegroups.com
Oh, sorry, Gmail didn't alert me to the last handful of messages
coming in while I was writing the above. So, +1 to Ambrose and Devin
from me.

Tassilo Horn

unread,
Jan 22, 2014, 11:35:40 AM1/22/14
to Alex Miller, cloju...@googlegroups.com
Alex Miller <al...@puredanger.com> writes:

Hi Alex,

> 2) "like defmacro" is intended to convey that a definline should not
> be expected to work as a HOF. Admittedly, that is subtle. :)

To me, the "like defmacro" conveys that it's syntax is like defmacro.

If definline didn't have the intention of also working as HOF, then it
would be equivalent to defmacro with the exception that it doesn't error
at compile-time (or at least runtime) when the definline-defined
function is used as a HOF. It should have been named
defmacro-with-hiding-of-programmer-errors in that case.

If one shouldn't call a definline function as HOF, then why not change
its definition to?

(defmacro definline
"Experimental - like defmacro, except defines a named function whose
body is the expansion, calls to which may be expanded inline as if
it were a macro. Cannot be used with variadic (&) args."
{:added "1.0"}
[name & decl]
(let [[pre-args [args expr]] (split-with (comp not vector?) decl)]
`(do
(defn ~name ~@pre-args ~args
(throw (RuntimeException. "DON'T CALL A DEFINLINE FN AS HOF!")))
(alter-meta! (var ~name) assoc :inline (fn ~name ~args ~expr))
(var ~name))))

Or make definline an alias for defmacro (if that's possible).

In both cases, calls like (map my-definline coll) would throw at
compile-time and prevent what you consider a misuse.

> The title of CLJ-1227 ticket is "Definline functions do not work as
> higher-order functions when AOT compiled". Per #2, don't do that and
> per #1, at your peril.

The problem is that even when *I* don't use definline (or only use it
non-HOF-y), some of my *dependencies* might do. And in that case, my
library or application will malfunction when it is AOT-compiled, because
AOT-compilation also AOT-compiles all dependencies. And AOT-compilation
is common at least for stand-alone applications and libs that need to be
callable from Java. Oh, and of course for production systems!

And the author of that dependency causing my trouble is not even the
culprit, because the bug only occurs with AOT-compilation, so normal
unit testing with clojure.test won't find it.

> While I appreciate that there is a patch on the ticket, the decision
> in this case was to not spend time on this issue.

Just by answering my mail and comment on the issue, you've already spend
more time on this issue than what would have been required to review and
apply my patch, or to come up with some other solution.

> Rich sees compiler macros as a strictly better solution than definline
> and would prefer to move in that direction for this kind of
> functionality.

I agree with him. Nevertheless, that some feature will eventually be
removed and superseded by something better does not mean that we should
simply ignore the bugs it has.

As said, not stuffing that hole in one way or another is grossly
negligent. More people than Colin and me will get bitten, and they get
bitten even harder when the faulty definline code is not in there own
stuff but somewhere in a dependency. And in both Colin's an my case,
the effect of the issue were not errors but wrong results which are much
harder to debug.

> I don't personally see that changing the word "Experimental" to
> "Deprecated" in the doc string would change anything.

I partly agree, because once you know how a function/macro works, you
don't check the docstring again. However, it might make a little
difference for those that think about using definline right now.

But anyway, IMO just rephrasing the docstring is not enough.

> To me those both mean, you probably shouldn't use this unless you're
> aware of the ramifications.

I'm pretty sure nobody has been aware of the fact that definline behaves
differently in AOT-compiled code than in non-AOT-compiled code.

Bye,
Tassilo

Gary Trakhman

unread,
Jan 22, 2014, 11:45:33 AM1/22/14
to cloju...@googlegroups.com
Just to add 2 cents.. I've depended on the HOF behavior of definline and ended up noticing something was wrong, but I wasn't able to figure out the problem.  1.5 years later, with this thread, I finally do :-).



--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

Bronsa

unread,
Jan 22, 2014, 11:53:21 AM1/22/14
to clojure-dev
FWIW http://dev.clojure.org/jira/browse/CLJ-1330 is the root-cause of this issue, fixing this (should be relatively easy) will have the side effect of fixing the definline issue without modifying its current definition.

Ambrose Bonnaire-Sergeant

unread,
Jan 22, 2014, 11:55:29 AM1/22/14
to cloju...@googlegroups.com
Oh that's interesting Nicola. That looks like exactly the reason why jvm.tools.analyzer chokes on analyzing clojure.core.

Thanks,
Ambrose

Tassilo Horn

unread,
Jan 22, 2014, 1:29:53 PM1/22/14
to Stuart Halloway, cloju...@googlegroups.com
Stuart Halloway <stuart....@gmail.com> writes:

> Devin: a patch that made the documentation string more clear would be ok.
>
> Tassilo: As already stated, the defect here is poor communication
> about limitations. Certainly we should communicate that what you want
> does not work, and will not be made to work.

As said, it does work unless you AOT-compile.

> I am mildly amused that you acknowledge not completely understanding
> the issue, but characterize the actions of those who do understand it
> as "insanity".

Sorry, I'm no native English speaker and couldn't find a better word
when I wrote that. I've meant it's "negligence" in the case the issue
is simply ignored. I don't care if it's dealt with it using my patch or
any other approach as long as it hinders users from falling into the
same trap as Colin an me.

And in that respect, I'd prefer a more drastic approach than just making
the docstring clearer. For what it's worth, I've used code like

(definline some-class? [obj]
`(instance? SomeClass ~obj))

(filter some-class? objs)

happily in the past, because it just works as I have expected. Well,
unless you AOT-compile...

Bye,
Tassilo

Tassilo Horn

unread,
Jan 22, 2014, 1:32:07 PM1/22/14
to Bronsa, clojure-dev
Bronsa <brob...@gmail.com> writes:

> FWIW http://dev.clojure.org/jira/browse/CLJ-1330 is the root-cause of this
> issue, fixing this (should be relatively easy) will have the side effect of
> fixing the definline issue without modifying its current definition.

Excellent! So let's try to get that fix (or another fix for it) into
1.6, please.

Bye,
Tassilo

Alex Miller

unread,
Jan 22, 2014, 1:42:31 PM1/22/14
to cloju...@googlegroups.com
That is interesting, particularly if it fixes issues in a broader AOT scope. It will probably be a day or two before I can look at it.

Alex   


Mark Engelberg

unread,
Jan 22, 2014, 2:17:17 PM1/22/14
to clojure-dev
When teaching students about Clojure macros, I've always taught that macros have several limitations with respect to functions, for example, you can't pass a macro to a higher-order function like map/filter/etc.

I then go on to say, "But there's something in Clojure called definline.  definline is interesting because it creates both a function and a macro.  It uses the macro every place it can, but when you use it in a context where a macro can't be used (like as an input to a higher-order function), it will use the function.  This is good for situations where you want it to work just like a function, but prefer for it be expanded ahead of time for performance and efficiency like a macro."

If definline doesn't really behave this way, then I was certainly misled by the documentation all these years.

Colin Fleming

unread,
Jan 22, 2014, 6:18:24 PM1/22/14
to cloju...@googlegroups.com
This is great Nicola, thanks for the investigation. I couldn't come up with a convincing explanation of why this happened (or why Tassilo's fix worked).

Devin Walters

unread,
Jan 22, 2014, 9:08:33 PM1/22/14
to cloju...@googlegroups.com, cloju...@googlegroups.com
+1, great sleuthing. Thanks for your work.

'(Devin Walters)

Mikera

unread,
Jan 23, 2014, 12:23:31 AM1/23/14
to cloju...@googlegroups.com
I don't think this is just "poor communications about limitations". If definline limitations are real and causing people problems, a better solution (by far) is to fix the limitations, not to improve the documentation of them. 

Tassilo has done at least 3 valuable things for the community here:
1) Highlighted a real issue, that has affected at least a few people 
2) Provided some helpful initial analysis and a patch. Even if this isn't the final answer, it's a helpful step along the way.
3) Pointed out that the issue was declined without a good reason (which is presumably a process problem that we can fix)

FWIW I think I've been bitten by this problem too, but never figured out the root cause (probably because I couldn't reliably reproduce at the REPL!). So thanks Tassilo! I appreciate your help in getting this noticed and resolved.

Shogo Ohta

unread,
Jan 23, 2014, 2:36:49 AM1/23/14
to cloju...@googlegroups.com
I feel what made this issue more cumbersome was not only that users relied on an experimental API, but that non-experimental core APIs did so.

Tassilo Horn

unread,
Jan 23, 2014, 2:59:18 AM1/23/14
to Shogo Ohta, cloju...@googlegroups.com
Shogo Ohta <atho...@gmail.com> writes:

> I feel what made this issue more cumbersome was not only that users
> relied on an experimental API, but that non-experimental core APIs did
> so.

My reasoning has also been that when definline is used by clojure core,
then it can't be so experimental that I should avoid it. And it's
trivial to migrate from definline to either defmacro or defn (possibly
with :inline metadata), if it would get removed.

But at the same time I have to confess that using the clojure core
definlines as HOFs (which exhibit the problem) is pointless as others
already mentioned. But then, why are doubles, floats, ints and friends
definlines instead of just macros?

Bye,
Tassilo

Alex Miller

unread,
Jan 24, 2014, 11:09:22 AM1/24/14
to cloju...@googlegroups.com
There is consensus that this is a problem, however this is an area of the code with broad impacts as it deals with how classes are named. To that end, there is some work that needs to be done in understanding the impacts before we can consider it.

Some questions we would like to answer:

1) According to Rich, naming of (fn x []) function classes used to work in the manner of this patch - with dynamically generated names. Some code archaeology needs to be done on why that was changed and whether the change to the current static behavior was addressing problems that we are likely to run into.

2) Are there issues with recursive functions? Are there impacts either in AOT or non-AOT use cases? Need some tests.

3) Are there issues with dynamic redefinition of functions? With the static naming scheme, redefinition causes a new class of the same name which can be picked up by reload of classes compiled to the old definition. With the dynamic naming scheme, redefinition will create a differently named class so old classes can never pick up a redefinition. Is this a problem? What are the impacts with and without AOT? Need some tests.

I would welcome help in these areas on CLJ-1330. 

Alex
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages