Done.
@flowthing Is there anything else you would like to do before we merge it?
Well, I wanted to give coal mine one last go on my own laptop. There's a run ongoing right now, let's see how it goes. Aside from that, I don't think there's anything left to do. We can make the non-set args change later if necessary, I think.
I got coal-mine to run 🙌 it'll take me a while to sort through the logs, though.
@flowthing looks good to me! As I mentioned in the PR, if you could squash the PR into one commit (or a couple if that makes more sense to you), I’d be very happy.
Also, I’m happy that you’ve taken the approach that the set fns should receive and return sets.
Sure thing, I have no problem squashing the whole thing.
@slipset I agree on the set in and out
Heh, my coal mine run eventually died with this:
rlwrap(1622,0x10e2db5c0) malloc: Incorrect checksum for freed object 0x7fd3cd001000: probably modified after being freed.
Corrupt value: 0xa700
rlwrap(1622,0x10e2db5c0) malloc: *** set a breakpoint in malloc_error_break to debug
clojure: Oops, crashed (caught SIGABRT) - this should not have happened!
If you need a core dump, re-configure with --enable-debug and rebuild
Resetting terminal and cleaning up...
Should've probably used clojure
instead of clj
.
Yeah, I think you are right Eero. That looks like rlwrap
itself died.
Yep. I tried clojure
and I got a successful run.
>Ran 50004 tests containing 191647 assertions. >0 failures, 782 errors.
kewl!
Here's the log file (~5 Mb): https://gist.github.com/eerohele/416de832a832d64573a81942eae05fc1
I just skimmed some of the clojure.set related failures. Every one I've seen so far has been a case of passing non-set args. A quick grep says there are almost 2000 clojure.set related hits, so, uh...
I'll probably need to try to come up with some way of filtering the log.
That's very cool! A bit like Hoogle for Clojure. 🙂
Right!
If I got my regexp right, 559 of the 782 failures are clojure.set related. I'll try to sort through at least some of them later.
curl <https://gist.githubusercontent.com/eerohele/416de832a832d64573a81942eae05fc1/raw/75285f458af58a2dd6305ddbecba14a1b1065424/log.txt> | grep -a2 "Call to #'clojure.set"
should give you most of them559 calls
that’s what I got too with this
Some interesting stuff here... for example, (set/subset "H" #{\! \, \.})
lol
if you feel bored, you can submit a PR to coal-mine to remove these 559 examples
Ehh... 😛
$ clj -Aspeculative --args 'inc [1 2 3]' -r '[2 3 4]' -e
(clojure.core/map)
😎can’t wait to try this with your set functions 😉
Heh. 🙂
A lot of cases of people calling set/union
with map args here.
probably for the same reason people call merge
instead of conj all the time. they just try stuff, and if it works they’re happy and keep using this pattern
Yep.
In contrast, I haven’t found any such error in advent-of-cljc so far
New feature:
$ clj -Aspeculative --args '"foo" 1' --ret 'string?' -v
| function | arguments | return value |
|-------------------+-----------+--------------|
| clojure.core/subs | "foo" 1 | "oo" |
| clojure.core/str | "foo" 1 | "foo1" |
Another project on core specs: https://github.com/gnl/ghostwheel.specs
Interesting, a lot of work done there.
Well, I went through the log. The overwhelming majority of cases is calls to set/union
with list or vector args. I didn't spot anything that seemed to be an error in the specs.
So if you're happy with the current state of the PR, as far as I'm concerned, you can push the "Merge and Squash" button, or I can squash it manually tomorrow.
Off to sleep now...
done
$ clj -Aspeculative --args 'nil' --ret 'nil' -e -v
| function | arguments | return value |
|--------------------------+-----------+--------------|
| clojure.set/intersection | nil | nil |
| clojure.core/first | nil | nil |
| clojure.core/merge | nil | nil |
| clojure.set/difference | nil | nil |
| clojure.set/union | nil | nil |
it works 🙂
If you have data and analysis of set fn calls in the wild, please add to that ticket in CLJ
$ clj -Aspeculative --args '#{1 2} #{2 3}' --ret 'set?' -v
| function | arguments | return value |
|--------------------------+---------------+--------------|
| clojure.set/intersection | #{1 2} #{2 3} | #{2} |
| clojure.set/difference | #{1 2} #{2 3} | #{1} |
| clojure.set/union | #{1 2} #{2 3} | #{1 3 2} |
| clojure.set/select | #{1 2} #{2 3} | #{2} |
Frequency data of important cases would be most useful
Going through the data of 4clojure, it’s mostly garbage calls that could be replaced by conj probably. But now that we have it in speculative, we may find more “real life” cases and I’ll definitely report them
Well I’m interested in frequent wrong cases too
The thing that happens frequently in 4clojure data is that merge is called as a conj replacement.
How does that relate to clojure.set?
@flowthing is probably detecting the same stuff: https://clojurians.slack.com/archives/CDJGJ3QVA/p1543698570029200
if you’re interested:
curl <https://gist.githubusercontent.com/eerohele/416de832a832d64573a81942eae05fc1/raw/75285f458af58a2dd6305ddbecba14a1b1065424/log.txt> | grep -a2 "Call to #'clojure.set"
will give you an impressionFunny, I detected one spec error in an advent of code solution by myself. None of the other solutions (48) gave problems.
Here I’m calling set/difference
with a key-seq instead of a set:
https://github.com/borkdude/advent-of-cljc/blob/master/src/aoc/y2017/d07/borkdude.cljc#L42
it wasn’t on purpose, just something that happened to work
I guess the second and others arg don’t have to be a set, as long as the first one is, because it uses reduce + disj
something that might have to be changed to the spec, or maybe we can agree that we should not rely on it
This is a case where coercing to a set also is unnecessary
The only other place off the top of my head where a set fn isn’t called with a set if also with KeySeqs: https://github.com/clojure/clojure/blob/master/src/clj/clojure/data.clj#L118
but I would be happy to change my advent of code solution to fit the spec