Clojure

Converted from http://www.assembla.com/spaces/clojure/tickets/250
Attachments:
add-clojure-debug-flag.patch - https://www.assembla.com/spaces/clojure/documents/aUWn50c64r35E-eJe5aVNr/download/aUWn50c64r35E-eJe5aVNr
assert-report-locals.patch - https://www.assembla.com/spaces/clojure/documents/aUWqLSc64r35E-eJe5aVNr/download/aUWqLSc64r35E-eJe5aVNr

Ignore the old patches. Considering the following implementation, please review and then move ticket to waiting on Stu:

1. RT will check system property "clojure.debug", default to false
2. property will set the root binding for the current *assert*, plus a new *debug* flag. (Debug builds can and will drive other things than just asserts.)
3. does Compile.java need to push *assert* or *debug* as thread local bindings, or can they be root-bound only when compiling clojure?
4. will add *debug* binding to clojure.main/with-bindings. Anywhere else?
5. build.xml should not have to change – system properties will flow through (and build.xml may not be around much longer anyway)
6. once we agree on the approach, I will ping maven plugin and lein owners so that they flow the setting through
7. better assertion messages will be a separate ticket
8. what is the interaction between *debug* and *unchecked-math*? Change checks to (and *unchecked-math* (not *debug*))}?

#3 - root bound only
#4 - should not be in with-bindings for same reason as #3 - we don't want people to set! *debug* nor *assert*
#8 - yes, wrapping that in a helper fn

#6 - my biggest reservation is that this isn't yet informed by maven best practices

System properties can be passed through Maven, so I do not anticipate this being a problem.

However, I would prefer *assert* to remain true by default.

SS is correct about this approach not posing any issue for Maven. In addition, the build could easily be set up to always emit two jars, one "normal", one "debug".

I'd suggest that, while clojure.debug might have broad effect, additional properties should be available to provide fine-grained control over each of the additional "debug"-related parameterizations that might become available in the future.

I'd like to raise a couple of potentially tangential concerns (after now thinking about assertions for a bit in the above context), some or all of which may simply be a result of my lack of understanding in various areas.

Looking at where assert is used in core.clj (only two places AFAICT: validating arguments to derive and checking pre- and post-conditions in fn), it would seem unwise to make it false by default. i.e. non-Named values would be able to get into hierarchies, and pre- and post-conditions would simply be ignored.

It's my understanding that assertions (talking here of the JVM construct, from which Clojure reuses AssertionError) should not be used to validate arguments to public API functions, or used to validate any aspect of a function's normal operation (i.e. "where not to use assertions"). That would imply that derive should throw IllegalArugmentException when necessary, and fn pre- and post-conditions should perhaps throw IllegalStateException – or, in any case, something other than AssertionError via assert. This would match up far better with most functions in core using assert-args rather than assert, the former throwing IllegalArgumentException rather than AssertionError.

That leads me to the question: is assert (and *assert*) intended to be a Clojure construct, or a quasi-interop form?

If the former, then it can roughly have whatever semantics we want, but then it seems like it should not be throwing AssertionError.

If the latter, then AssertionError is appropriate on the JVM, but then we need to take care that assertions can be enabled and disabled at runtime (without having to switch around different builds of Clojure), ideally using the host-defined switches (e.g. -ea and friends) and likely not anything like *assert*. I don't know if this is possible or practical at this point (I presume this would require nontrivial compiler changes).

Hopefully the above is not water under the bridge at this point. Thank you in advance for your forbearance.

Thanks for the useful input Chas. Nothing is concluded yet. I think we should step back and look at the objective here, before moving forward with a solution. Being a dynamic language, there are many things we might want to validate about our programs, where the cost of checking is something we are unwilling to pay in production.

Being a macro, assert has the nice property that, should *assert* not be true during compilation, it generates nil, no conditional test at all. Thus right now it is more like static conditional compilation.

Java assert does have runtime toggle-ability, via -ea as you say. I haven't looked at the bytecode generated for Java asserts, but it might be possible for Clojure assert to do something similar, if the runtime overhead is negligible. It is quite likely that HotSpot has special code for eliding the assertion bytecode given a single check of some flag. I'm just not sure that flag is Class.desiredAssertionStatus.

Whether this turns into changes in assert or pre/post conditions, best practices etc is orthogonal and derived. Currently we don't have a facility to run without the checks. We need to choose between making them disappear during compilation (debug build) or runtime (track -ea) or both. Then we can look at how that is reflected in assert/pre-post and re-examine existing use of both. The "where not to use assertions" doc deals with them categorically, but in not considering their cost, seems unrealistic IMO.

I'd appreciate it if someone could look into how assert is generated and optimized by Java itself.

Bytecode issues continue to be above my pay grade, unfortunately…

A few additional thoughts in response that you may or may not be juggling already:

assert being a macro makes total sense for what it's doing. Trouble is, "compile-time" is a tricky concept in Clojure: there's code-loading-time, AOT-compile-time, and transitively-AOT-compile-time. Given that, it's entirely possible for an application deployed to a production environment that contains a patchwork of code or no code produced by assert usages in various libraries and namespaces depending upon when those libs and ns' were loaded, AOT-compiled, or their dependents AOT-compiled, and the value of *assert* at each of those times. Of course, this is the case for all such macros whose results are dependent upon context-dependent state (I think this was a big issue with clojure.contrib.logging, making it only usable with log4j for a while).

What's really attractive about the JVM assertion mechanism is that it can be parameterized for a given runtime on a per-package basis, if desired. Reimplementing that concept so that assert can be *ns*-sensitive seems like it'd be straightforward, but the compile-time complexity already mentioned remains, and the idea of having two independently-controlled assertion facilities doesn't sound fun.

I know nearly nothing about the CLR, but it would appear that it doesn't provide for anything like runtime-controllable assertions.

The best (dated) evidence I could find says that the compiler sets a special class static final field $assertionsDisabled based on the return of desiredAssertionStatus. HotSpot doesn't do anything special with this, dead code elimination simply makes it go away. The code indeed compiles this way:

11: getstatic #6; //Field $assertionsDisabled:Z
14: ifne 33
17: lload_1
18: lconst_0
19: lcmp
20: ifeq 33
23: new #7; //class java/lang/AssertionError
26: dup
27: ldc #8; //String X should be zero
29: invokespecial #9; //Method java/lang/AssertionError."<init>":(Ljava/lang/Object;)V
32: athrow

Even if we were 100% sure that assertion removal was total, I would still vote for a separate Clojure-level switch, for the following reasons:

1. I have a real and pressing need to disable some assertions, and I don't need the Java interop at all. Arguably others will be in the same boat.
2. there will be multiple debugging facilities over time, and having a top-level debug switch is convenient for Clojure users.
3. Java dis/enabling via command line flags is still possible as a separate feature. We could add this later as a (small) breaking change to our assert, or have a separate java-assert interop form. I am on the fence about which way to go here.
4. I believe it is perfectly fine to throw an AssertionError from a non-Java-assertion-form. We don't believe in a world of a static exception hierarchy, and an assertion in production is a critical failure no matter what you call it. Even Scala does it http://daily-scala.blogspot.com/2010/03/assert-require-assume.html

Rich: awaiting your blessing to move forward on this.

The compiler sets $assertionsDisabled when, in static init code? Is there special support in the classloader for it? Is there a link to the best dated evidence you found?

1. Yes, in static init code
2. There is no special support in the classloader, per Brian Goetz (private correspondence) last week. But dead code elimination is great: "The run-time cost of disabled assertions should indeed be zero for compiled code"

Link: Google "java assert shirazi". (Not posting link because I can't tell in 10 secs whether it includes my session information.)

Is there anything new on this issue? I also look for a convenient way to disable assertions in production.