nextjournal / clerk

⚡️ Moldable Live Programming for Clojure
https://clerk.vision
ISC License
1.74k stars 75 forks source link

An edge case where the analyzer crashes #628

Closed zampino closed 4 months ago

zampino commented 5 months ago
(ns scratch.repro
  {:nextjournal.clerk/no-cache true})

(do
  (declare b)
  (def a 1))

(inc a)

Throws

Execution error (ExceptionInfo) at nextjournal.clerk.analyzer/hash-codeblock (analyzer.clj:565).
`->hash` must be `ifn?`

from

https://github.com/nextjournal/clerk/blob/65988e062c8e9dc5e48111d05fec5350cf80e4c9/src/nextjournal/clerk/analyzer.clj#L562-L568

I find this error pretty confusing, as we should rather check that all dependencies have been hashed at this point, and not that ->hash is just a map. This could allow that certain deps are not hashed and that the nil value lands in the hashed-deps set above.

The error above reproduces only if the node a is at the beginning of the list (hence precedes declare) in the topological sorting of the analyzer graph nodes, this is the not all the time the case, apparently.

This issue is due to the fact that we store the form

(do
  (declare b)
  (def a 1))

under the key a in the analysis info map, while it probably should be stored under the block id only.