Fork me on GitHub
#clojure
<
2021-12-13
>
h0bbit04:12:55

^ This is what we ended up putting in our playbooks for guarding against the CVE. Works nicely!

genRaiy12:12:59

when reading the clojure core specs I noticed this oddity works

(defn fn-with-attr-map
  "Is a fine function"
  {:some-meta-here true}
  ([a b]
   (+ a b))
  ([a b c]
   (+ a b c))
  {:some-extra-meta true})

genRaiy12:12:34

the last map is merged with the first to produce the metadata

genRaiy12:12:38

I've never seen it used but my reading of the Clojure code is limited 🙂 ... any happy users of this facility out there?

vlaaad12:12:28

Oh I remember Alex Miller explaining rationale behind this before... Unfortunately, don't remember the explanation

reefersleep13:12:59

Hah, that’s odd. At a glance, I thought that the extra stuff was lexically attached to one of the arities.

Alex Miller (Clojure team)13:12:55

It's attached to the var, not the arities

Alex Miller (Clojure team)13:12:28

It's allowed at the end for the case where you might have a big metadata map and don't want to obscure the arities

1
🙏 1
Alex Miller (Clojure team)13:12:45

But it is very rarely used

borkdude15:12:11

Would it be reasonable to warn against using quoted test constants in case for clj-kondo or is there a legitimate use case for it? https://github.com/clj-kondo/clj-kondo/pull/1491#issuecomment-992576345 The following returns :quote-a , as expected (but not everyone might expect that). Another interesting bit, if you uncomment the #{a} one, it seems the entire case is expanded into a condp = and it fails with unable to resolve a .

(prn (case 'quote
       ;; #{a} 4
       ;; a :a
       'a :quote-a ;; duplicate
       ;; '[a] :foo
       ;; [a] :bar ;; duplicate
       1))

🤯 1
dgr15:12:47

IMO it’s reasonable to warn. It’s more likely to be an error on the programmer’s part than a legitimate need. If the programmer really wants to match against quote, they can write it out (and IMO they should).

1
noisesmith19:12:00

yes, given that 'a and (quote a) are the same, it makes sense to direct them to the expanded version

borkdude20:12:44

OK, the issue is implemented in clj-kondo: https://twitter.com/borkdude/status/1471936924483006475 by @U02QXKSHP97 (thanks!) I just wonder if the warning could become more clear to explain what the issue is. Currently users might still think: yeah, quoting, that's what I want dude, what are you complaining about.

borkdude20:12:39

Maybe we should emphasize: > The test-constants are not evaluated. They must be compile-time > literals, and need not be quoted.

borkdude20:12:12

So the message could become: "Test-constants are not evaluated and need not be quoted: 'a"

Colin P. Hill15:12:40

Asking in a separate message rather than the thread, so as not to drive that conversation away from borkdude's actual question: how in the world is that working? Why does 'quote match the 'a branch?

borkdude15:12:15

@dorandraco Because 'a is read as (quote a)

borkdude15:12:51

user=> (defmacro view [x] (prn x))
#'user/view
user=> (view 'a)
(quote a)
nil

borkdude15:12:23

And then you get: (case x (quote a) :quote-or-a)

borkdude15:12:54

This is why many people get away without knowing symbols need not be quoted, they are actually matching agains the symbol quote as well

Colin P. Hill15:12:13

Oh! Because case interprets lists as alternatives, not as calls. That's super confusing.

borkdude15:12:58

clj-kondo can actually see if you wrote 'a vs (quote a) so it can warn appropriately on the first thing

Jan K15:12:44

The "unable to resolve" error with #{a} is most puzzling for me, even though eg.`#{b}` works. Surely it doesn't turn into condp, it's supposed to be constant-time dispatch

borkdude15:12:21

It seems case falls back to condp when there are too may different types:

user=> (case '#{a} #{a} 1)
1
user=> (case '#{a} #{a} 1 :foo 2)
1
user=> (case '#{a} #{a} 1 :foo 2 'a 3)
Syntax error compiling at (REPL:1:1).
Unable to resolve symbol: a in this context

borkdude15:12:10

or no, it contains a condp=.

user=> (macroexpand '(case '#{a} #{a} 1 :foo 2 'a 3))
(let* [G__168 (quote #{a})] (case* G__168 9 3 (throw (java.lang.IllegalArgumentException. (clojure.core/str "No matching clause: " G__168))) {1 [-1640525200 (clojure.core/condp clojure.core/= G__168 #{a} 1 a 3 (throw (java.lang.IllegalArgumentException. (clojure.core/str "No matching clause: " G__168))))], 2 [:foo 2], 3 [quote 3]} :compact :hash-equiv #{1}))

ghadi18:12:38

@borkdude case uses condp not when there are too many different types, but when hashes collide (and it has chosen the 'hash-equiv' strategy)

ghadi18:12:14

the pseudo code for it is: x = (hash the case input argument) jumpidx = (clojure.core/shift-mask shift mask x) jump to that index in the bytecode table make sure that the input and the table entry are =

borkdude18:12:05

should the condp fragment quote the set in (case '#{a} #{a} 1 :foo 2 'a 3) ?

ghadi18:12:21

this example is super confusing... I'm still trying to mentally figure it out 🙂

ghadi18:12:48

user=> (#'clojure.core/shift-mask 9 3 (.hashCode '#{a}))
1
user=> (#'clojure.core/shift-mask 9 3 (.hashCode 'quote))
3
user=> (#'clojure.core/shift-mask 9 3 (.hashCode 'a))
1
user=> (#'clojure.core/shift-mask 9 3 (.hashCode :foo))
2

ghadi18:12:09

those are the 4 branches in the example, note two clashes

borkdude18:12:54

but it seems the condp expansion isn't quoting the set, which causes an error?

borkdude18:12:16

note that this is a totally fabricated example, not one from production code, but still

quoll19:12:13

The 'a is a bit confusing, because in any other context it would be a single value, but in a case it becomes a pair. But I agree with Michiel, the condp isn’t quoting the values that it receives. It’s not just a missing quote on that set. It also affects the symbol a:

user=> (case 'a #{a} 1 :foo 2 a 3)
Syntax error compiling at (REPL:1:1).
Unable to resolve symbol: a in this context
user=> (def a nil)
#'user/a
user=> (case 'a #{a} 1 :foo 2 a 3)
Execution error (IllegalArgumentException) at user/eval158 (REPL:1).
No matching clause: a
If I hand-edit the code generated from the case macro so that the compared values in the condp are quoted, then it works as intended.

quoll20:12:17

I’ve put a suggested patch on https://ask.clojure.org/index.php/11395/hash-collisions-case-statements-try-evaluate-the-constants, since I think it has to go through there before it can go to Jira, right?

kanwei21:12:16

How do I do type hinting for reflection for things like String[] and File[]?

chancerussell21:12:42

^"[Ljava.lang.String;"

dorab21:12:47

Ljava.lang.String;

dorabs-imac:~ dorab$ clj
Clojure 1.10.3
user=> (type (into-array String []))
[Ljava.lang.String;
user=> 

chancerussell22:12:50

You need the quotes because of metadata rules, you need the [ because it’s an array, and you need the ; because it’s a reference type

kanwei22:12:08

that's some wild syntax, thanks!

quoll23:12:52

Unrelated to @U0ELT5ZDE’s question, this is an interesting rabbithole to go down. That link above is for ClassNames in internal form for the JVM. That’s actually different to the form of BinaryNames described in the Java Language Specification. Quoting from the JVM spec: > For historical reasons, the syntax of binary names that appear in `class` file structures differs from the syntax of binary names documented in JLS §13.1. In this internal form, the ASCII periods (`.`) that normally separate the identifiers which make up the binary name are replaced by ASCII forward slashes (`/`). The identifiers themselves must be unqualified names (https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.2.2). The Java Language Specification https://docs.oracle.com/javase/specs/jls/se17/html/jls-13.html#jls-13.1 is a bit harder to read than the JVM spec document, so it makes sense to see what they look like in the JVM spec, and just replace the / characters with . characters.

💯 2
quoll23:12:50

You can also get to these links by looking at the javadoc for https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Class.html#getName(). This says that it will return the binary name of the class with a https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/ClassLoader.html#binary-name. This describes class names with dot separators, and has links to the Java Language Specification (https://docs.oracle.com/javase/specs/jls/se17/html/jls-6.html#jls-6.7 and https://docs.oracle.com/javase/specs/jls/se17/html/jls-13.html#jls-13.1)

noisesmith00:12:44

@U07MMB5JB close - you need the double quotes because otherwise it wouldn't be readable (unbalanced [ token and trailing ; token break reader rules)