Clojure 1.10 issue: thrown? no longer works on macros

[Mark Engelberg]

Consider the following macro:

(defmacro f [x] {:pre [(number? x)]} `(+ ~x 5))
=> (f 3)
8
=> (f true)
Unexpected error (AssertionError) macroexpanding f at (test:localhost:62048(clj)*:265:28).
Assert failed: (number? x)

So, as expected it throws an AssertionError if passed a non-number.

However, the following test (using is from clojure.test) used to work prior to 1.10, but now fails:

=> (is (thrown? AssertionError (f true)))

Unexpected error (AssertionError) macroexpanding f at (test:localhost:62048(clj)*:268:56).
Assert failed: (number? x)

What's odd is that the macro still throws an AssertionError, but the `thrown?` inside the `is` is no longer intercepting the AssertionError, so the test doesn't pass -- instead the error causes a failure in the test suite.

[alex]

I'm not sure, but probably it behaves so because of throwing at macroexpand stage.

вторник, 18 декабря 2018 г., 11:29:09 UTC+2 пользователь puzzler написал:

[Mark Engelberg]

Agreed.  It is not a problem for functions which throw AssertionErrors, only macros.  But this is a change in behavior which breaks test suites which passed previously.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Alex Miller]

I think the relevant change here is that exceptions thrown during macroexpansion are now wrapped into CompilerExceptions as a way to attach all of the source context information. The REPL understands this and still prints the original cause so the printing hides some of that structure (but you can see it by looking at the exception chain). 

This here is a particularly tricky case here though and I think there might be something else coming into play, still looking at it. There was a couple other changes inside the compiler exception handling that might also be coming into play.

(Would have been great to see this during a pre-release build rather than after release! ;) 

[Alex Miller]

This particular example given fails in a similar way on 1.9. Could you give me something closer to what you are actually seeing in your test suite? Specifically something that is a passing clojure.test test on 1.9 but a failure/error on 1.10? 

The problem with the given example is that it fails during macroexpansion of the test itself, not during test execution, and I don't think that's the case you're trying to replicate.

[Alex Miller]

In particular, I am challenging this assertion from the original post:

However, the following test (using is from clojure.test) used to work prior to 1.10, but now fails:

=> (is (thrown? AssertionError (f true)))

Unexpected error (AssertionError) macroexpanding f at (test:localhost:62048(clj)*:268:56).

Assert failed: (number? x)

When I do this on 1.9 I see:

user=> (is (thrown? AssertionError (f true)))

CompilerException java.lang.AssertionError: Assert failed: (number? x), compiling:(NO_SOURCE_PATH:20:29)

Which is printed differently but is the same behavior, because this is happening while expanding the is, not during execution.

I believe you're seeing a change in your test suite, I'm just having a hard time reproducing what you're seeing.

[Andy Fingerhut]

I do not know if Leiningen is involved in the difference in the reproduction project linked below yet (I am testing with the latest Leiningen 2.8.3 here), but I do see a clojure.test `(is (thrown-with-msg? ...` expression that gives no error when running 'lein test' with Clojure 1.9.0, but does with Clojure 1.10.0.  I do not know if it is the same root cause as Mark's example or not, but wanted to create this in hopes it represents a minimal test case to reproduce the behavior difference.

https://github.com/jafingerhut/clojure-110-is-thrown

Andy

--

[Alex Miller]

Leiningen monkey patches clojure.test so I tend not to trust it for repros.

You received this message because you are subscribed to a topic in the Google Groups "Clojure" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure/b2fdyfhFBrg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojure+u...@googlegroups.com.

[Alex Miller]

Oh, if you're testing with thrown-with-msg?, that definitely can have an impact due to the nested exception. In clojure.test, I'm using a variant to test the message with the root cause:

https://github.com/clojure/clojure/blob/master/test/clojure/test_helper.clj#L134-L151

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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

[Mark Engelberg]

Sorry, I tried to "shrink the error case" and I appear to have gone too far.  The actual problem would be more like:

=> (is (thrown? AssertionError (eval (quote (f true))))

clojure+u...@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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

For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+u...@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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

[Mark Engelberg]

And to be as specific as possible, the actual test that worked in 1.9 but not in 1.10 is:

https://github.com/Engelberg/instaparse/blob/dcfffad5b065e750f0f5835f017cdd8188b8ca2e/test/instaparse/defparser_test.cljc#L34

I've temporarily elided the test from the instaparse test suite, but wanted to report it.  Again, I apologize for not realizing the eval/quote was an important aspect and leading you down a rabbit hole.

[Mark Engelberg]

And the macro which that test case exercises can be found here:

https://github.com/Engelberg/instaparse/blob/dcfffad5b065e750f0f5835f017cdd8188b8ca2e/src/instaparse/core.cljc#L274

[Andy Fingerhut]

Alex, just so I understand, are you saying that because of the implementation of `(is (thrown-with-msg? ..))` and the changes to exceptions in Clojure 1.10.0, that other Clojure programs that use `(is (thrown-with-msg? ...))` may need to be updated to use something similar to what you linked, in order to test the new Clojure 1.10.0 behavior?

Thanks,

Andy

clojure+u...@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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

For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+u...@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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

[Alex Miller]

No, I'm not saying that - the reason I was looking at cause was due to some gnarly exception check messages for compiling Clojure code. I'm not sure that these are applicable in general. Given that this is not what the original case was, I think we should set that aside for now.

Going back to the original instaparse case (with thrown?), I reproduced that and then simplified it down to this: 

Pick a version:

  clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.9.0"}}}'

  clj -Sdeps '{:deps {org.clojure/clojure {:mvn/version "1.10.0"}}}'

Repro:

 

(use 'clojure.test)

(defmacro p [k] {:pre [(keyword? k)]} [])

(is (thrown? AssertionError (eval '(p 1))))

;; 1.9:  passes, returns the error

;; 1.10: fails test\

Note that there are several unique factors about this case, specifically that it involves a macro precondition, triggered in an eval, in a deftest "is". 

Just throwing an exception (no precondition) shows no change in behavior:

(defmacro p2 [] `(throw (NullPointerException. "")))

(is (thrown? NullPointerException (eval '(p2))))

;; same behavior in both 1.9 and 1.10 - passes and returns the exception

At this point, I'm moving discussion to a ticket and I'd rather pick up any subsequent discussion there:

https://dev.clojure.org/jira/browse/CLJ-2458

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to

clojure+unsubscribe@googlegroups.com

For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.

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