Welcome! Please see the About page for a little more info on how this works.

0 votes
in Clojure by
This ticket includes two patches:

#  a patch to set  {{*assert*}} when clojure.lang.RT loads, based on the presence of system property clojure.debug
# expand error messages in assert to include {{local-bindings</code> (a new macro which wraps the implicit <code>&env}})

Things to consider before approving these patches:

# should there be an easy Clojure-level way to query if debug is enabled? (checking assert isn't the same, as debug should eventually drive other features)
# assertions will now be off by default -- this is a change!
# is the addition of the name {{local-bindings}} to clojure.core cool?

15 Answers

0 votes
by

Comment made by: stu

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 , plus a new flag. (Debug builds can and will drive other things than just asserts.)
  3. does Compile.java need to push } or } as thread local bindings, or can they be root-bound only when compiling clojure?
  4. will add * } 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 }}?
0 votes
by

Comment made by: richhickey

#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

0 votes
by

Comment made by: stuart.sierra

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

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

0 votes
by

Comment made by: cemerick

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. (link: http://download.oracle.com/javase/1.4.2/docs/guide/lang/assert.html#usage text: "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 * }) 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 * . 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. ;-)

0 votes
by

Comment made by: richhickey

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 (link: http://download.oracle.com/javase/1.5.0/docs/api/java/lang/Class.html#desiredAssertionStatus() text: 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.

0 votes
by
_Comment made by: cemerick_

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.
0 votes
by

Comment made by: stu

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."":(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.

0 votes
by

Comment made by: richhickey

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?

0 votes
by

Comment made by: stu

  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"
0 votes
by

Comment made by: stu

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

0 votes
by

Comment made by: akiel

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

0 votes
by

Comment made by: lopusz

I am also interested in any news on this issue.
Convenient way to enable/disable assertions at runtime (preferably via -ea/-da options) would be a great feature!

0 votes
by

Comment made by: lopusz

Btw. there is a library for runtime-toggable assertions available via clojars
https://github.com/pjstadig/assertions
This helped me a great deal.

0 votes
by
...