JSON encoding #1469
No reviewers
Labels
No labels
UX
active development
backlog
blocker
bootstrap
bounty
bug
dependencies
discussion
documentation
duplicate
enhancement
flaky test
help wanted
invalid
javascript
question
release
tendentious
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
mighty-gerbils/gerbil!1469
Loading…
Reference in a new issue
No description provided.
Delete branch "v0.19-json"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
(cooked with Claude)
JSON encoding cooked with Claudeto JSON encodingJSON encodingto [WIP] JSON encoding9cd5549062to876262cb72e2bc4ba397to15ccaa4113[WIP] JSON encodingto JSON encoding15ccaa4113to3cbb331e55what is going on here? why is there a bootstrap? you need to retarget.
3cbb331e55to0df1cd4ea60df1cd4ea6tode5dc20b3dde5dc20b3dtoa99610c5d6first pass review, my eyes hurt from the slop and the clanker is shitting all over the codebase again.
can't you tell it to not ever change unrelated code? it's idiotic.
@ -43,2 +46,3 @@;; returns nibble (0..15) or #f if char is not a hex digit(let (i (char->integer char))(and (fx< i (u8vector-length unhexes))(and (fx< i 123) ;; 123 == (u8vector-length unhexes)please don't hardcode this, just use u8vector-length
@ -13,0 +7,4 @@:std/list/walist:std/encoding/hex./env)(export __parse-json)i think we should export all the component procedures (without __) so that unmarshallers for objects can use them.
@ -13,0 +18,4 @@;;; whitespace(def (__skip-whitespace (reader : BufferedReader))if it is not an unchecked contract, it shouldn't be __
this applies to all those procedures here.
@ -0,0 +36,4 @@(writer.write-json obj ctx)(get-memory-output-string-utf8 writer)))((and (output-port? writer) (not (binary-port? writer)))(display (write-json #f obj ctx) writer))you can open-buffered-reader on the port, no need for the in memory translation.
you should then buffer-detach! after being done.
@ -31,0 +34,4 @@;;;;;; escape callback for write-string/escape: returns :fixnum or #f(def (json-string-escape (writer :- BufferedWriter) (u8 :- :fixnum))this is incredibly verbose and repetitive. clanker code.
just generalize that, make a table for the escape chars and it's 1/10th the code.
@ -68,3 +68,3 @@result))))(['int arg how flags width](with-syntax ((ctx ctx) (arg arg) (how how) (flags flags) (width width) (writer writer))(with-syntax ((ctx ctx) (arg arg) (how how) (flags (datum->syntax ctx (list 'quote flags))) (width width) (writer writer))wtf
@ -78,3 +78,3 @@result))))(['float arg how flags width precision result](with-syntax ((ctx ctx) (arg arg) (how how) (flags flags) (width width) (precision precision) (writer writer))(with-syntax ((ctx ctx) (arg arg) (how how) (flags (datum->syntax ctx (list 'quote flags))) (width width) (precision precision) (writer writer))wtf
@ -84,1 +85,3 @@(begin(wr (__print-float str-buf fmt-buf num))(buflen (u8vector-length str-buf)))(condWTF. why does the clanker have to shit all over the code base?
You have to be more specific, or I can't fix it.
@ -47,3 +49,1 @@(let (read (DatagramSocket-recvfrom sock peer buffer))(DatagramSocket-sendto sock (unbox peer) buffer 0 read)(lp)))))(with-catchtry/catch
@ -341,0 +357,4 @@;;; Continuations are applied to s1's mismatch index;;;; in the case of equality, this is END1.(def (%string-compare s1 start1 end1 s2 start2 end2contracts
@ -0,0 +26,4 @@(declare (not safe))(defstruct evector ((vector : :vector)EVector or something
or ExtensibleVector
@ -0,0 +31,4 @@transparent: #tconstructor: :init!)(defstruct eu8vector ((u8vector : :u8vector)ExtensibleU8Vector or EU8Vector
@ -0,0 +36,4 @@transparent: #tconstructor: :init!)(defstruct ebits ((bits : :u8vector)here too
@ -0,0 +118,4 @@;;; %SMALLEST-LENGTH takes care of the type checking -- which is what;;; the CALLEE argument is for --; thus, the design is tuned for;;; avoiding redundant type checks.(define %smallest-lengthtry to make it proper gerbil, this is just copy past slop from srfi
below too.
a99610c5d6toe69afc63807cd4da41f0to97669fd51cI had to refactor format somewhat. Now passes all tests in both dynamic and static mode. I removed the numeric options from the FormatEnv, at least for now: it isn't needed at this point as the numeric options only apply to flonums, and not to e.g. lists of flonums. Instead, options are passed statically to the flonum handler.
4c335aa837to4d3022b7a2@ -264,6 +264,17 @@ namespace: #f1))(field-info-length (##fx* 3 (##fx- (##vector-length slot-vector) first-new-field)))(field-info (make-vector field-info-length #f))(__field-info-initslop pasta, just _
@ -338,0 +354,4 @@;; name = (vector-ref (##type-fields type) (fx* field-index 3));; flags = (vector-ref (##type-fields type) (fx+ (fx* field-index 3) 1));; default = (vector-ref (##type-fields type) (fx+ (fx* field-index 3) 2))(def (struct-field-fold klass kons knil)this is used in 1 place, and we already have a procedure that does what is needed there: class-type-printable-slots which caches it in the properties.
please remove this dr sloppenheimer.
@ -30,0 +42,4 @@(loop (fx1+ i)))unhexes))))(def length-unhexes% (u8vector-length unhexes%))) ;; 103(defsyntax unhexes (lambda (stx) unhexes%))dr sloppenheimer strikes again
this is completely wrong because it doesn't tell you when you use it wrong.
it is also completely unnecessary because you can (@eval unhexes)
even worse, you don't need this to be in syntax at all.
you already have a module that is imported for both, i suggest you put it there and it is available at runtime without the shenenigans.
we know the clanker doesn;t grok macros, so dont even try.
The point of doing it in syntax is precisely to avoid the computation at runtime. I simplified further.
@ -30,0 +43,4 @@unhexes))))(def length-unhexes% (u8vector-length unhexes%))) ;; 103(defsyntax unhexes (lambda (stx) unhexes%))(defsyntax length-unhexes (lambda (stx) length-unhexes%))and again.
@ -13,0 +72,4 @@(when key;; If you see a duplicate key, it's as likely an attack as a bug. #LangSec(if (key? key)(error "Duplicate hash key in JSON input" key)The War on Error is raging on.
it's an IOError, raise-io-error
or refactor ParseError from serde into std/error and raise that.
@ -13,0 +96,4 @@(when key;; If you see a duplicate key, it's as likely an attack as a bug. #LangSec(if (key? key)(error "Duplicate hash key in JSON input" key)WoE
@ -13,0 +119,4 @@(if key;; If you see a duplicate key, it's as likely an attack as a bug. #LangSec(if (key? key)(error "Duplicate hash key in JSON input" key)WoE
@ -13,0 +142,4 @@(if key;; If you see a duplicate key, it's as likely an attack as a bug. #LangSec(if (key? key)(error "Duplicate hash key in JSON input" key)WoE
@ -0,0 +110,4 @@(walist(with ([strukt . fields] (struct->list struct))(with-list-builder (c)(struct-field-foldclass-type-printable-slots
@ -31,0 +71,4 @@(fx+ n (if (fx< n 10) #x30 #x37)))(do-write (wr 0)(writer.write-backslash)(writer.write-u8 117) ; u(@eval (char->integer #\u)) is much cleaner, you can use the (@char->int char) macro from format.
we shouldn't be hardcoding numbers like that and have to check the ascii tables on review.
@ -42,0 +107,4 @@(def (write-json-float (writer : BufferedWriter)(obj :~ (? (and flonum? finite?)) :- :flonum)) => :fixnum(let ((s (number->string obj)))i know this is older code, but we should probably use the format flonum printer here as it avoids the ugly shit with number->string's behavior.
maybe the clanker can oblige, no creativity required.
Akshully, if you want to be 100% JSON-compliant, the edge cases are a bitch, and this works pretty well. For instance
(format "%.17g" obj)should work pretty well... as long as we operate under the C locale. Is that something we can guarantee? Probably not.@ -152,3 +188,1 @@;; builtin objects(defjson-writer :t (write-json-t writer obj env)(using (klass (class-of obj) :- :class)(def (write-json-object (writer : BufferedWriter) obj (ctx : JSONWriteContext)) => :fixnumideally this would be implementations of a JSONWriter interface so that users can do their own classes easily (with an auto implementation that just writes the printable slots).
it's ok for now.
@ -158,0 +224,4 @@(writer.write-string-utf8 "false"))((void? obj)(writer.write-string-utf8 "null"))((method-ref obj ':write-json)here a defcall-interface-method would be the prefered way to handle this.
again, this is for a later pr.
@ -69,2 +66,2 @@(['int arg how flags width](with-syntax ((ctx ctx) (arg arg) (how how) (flags flags) (width width) (writer writer))(['int arg how flags width precision](let (conv (case howdon't put this inline here like this, make a utility procedure dr sloppenheimer.
the trinity test is having way more fallout than expected.
in fact i think we already have a procedure that does this.
see __integer-conversions
@ -93,3 +96,3 @@(cons (case how((#\a) #'(format-display writer arg ctx))((#\s) #'(format-write writer arg ctx))((#\s #\w) #'(format-write writer arg ctx))is this %w a new fruit in the basket?
fix the indentation pls.
After careful consideration of what ~W means, I decided against adding it for now.
It's supposed to be there for pretty-printing, but until I implement that, it doesn't make sense to add it and have it mean just the same as %s.
@ -80,3 +79,3 @@(case how((#\a) (format-display writer arg fmt.env))((#\s) (format-write writer arg fmt.env))((#\s #\w) (format-write writer arg fmt.env))fix the indentation pls.
@ -351,0 +309,4 @@;; Write Scheme prefix (#x/#o/#b); contributes 0 if no # flag or decimal.(def (write-prefix!)(if prefix-char(begin (writer.write-sharp) (writer.write-u8 prefix-char) 2)why does the clanker insist on hardcoding things?
@ -36,3 +36,3 @@(fx+ i 1))))))(def (buffer-cache-get-float-format-buffer (opt : FormatOpt) (num : :flonum)) => :u8vector(def (buffer-cache-get-float-format-buffer flags width precision (flonum-repr :- :fixnum)) => :u8vectordont :- this
@ -64,0 +61,4 @@(int-part-reserve(if (or (fx= flonum-repr (@char->int #\f))(fx= flonum-repr (@char->int #\F)))313 0))what is 313?
@ -82,3 +82,1 @@(wr (__print-float str-buf fmt-buf num)))(defrule (release!)(begin(defwriter-ext (format-float writer (num : :flonum) flags width precision (flonum-repr :- :fixnum))the clanker completely slopified this procedure.
please fix.
Is it better now?
@ -85,0 +86,4 @@(buflen (u8vector-length str-buf)))(cond((fx<= wr 0)(buffer-cache.put! str-buf)please restore my release macro dr sloppenheimer.
the clanker is an idiot who wants to make all code unreadable slop.
@ -94,0 +96,4 @@(else;; Buffer too small — shouldn't happen with correct formula, but retry gracefully.(buffer-cache.put! str-buf)(let* ((str-buf2 (make-u8vector (fx+ wr 1)))why str-buf2?
Clanker wanted to retry if the buffer size had not been properly computed. I think it's better to just report all the data to reproduce the bug in the error message.
@ -125,3 +125,3 @@;; *one* possible key for each value.;; : (Table K V) <- (Table V K) to: (Optional (Table K V))(def (invert-hash (from : HashTable) (to : HashTable := (HashTable-new from)))(def (invert-hash (from : HashTable) to: (to : HashTable := (HashTable-new from)))why does a single optional argument need a keyword?
To have the same API as the other variants that have a lot more such arguments.
@ -139,2 +151,2 @@(done?(let (buf mem.buf)(if done?;; Take ownership of the buffer directly rather than copying + caching.i am having second thoughs about this, as it blows the cache.
@ -40,0 +108,4 @@(start :- :fixnum)(end :- :fixnum)(need :- :fixnum)(total :- :fixnum))what is this total shit? it doesn't satisfy the interface, BUG?
It was only used internally, but yes, poor clanker taste, and my bad for failing to catch that. Thanks. Refactoring this, and more, and adding test cases to make sure everything goes right.
@ -241,3 +241,1 @@(fxior (fxarithmetic-shift-left (fxand byte1 #x0f) 12)(fxarithmetic-shift-left (fxand byte2 #x3f) 6)(fxand byte3 #x3f))))(fxior (fxarithmetic-shift-left (fxand byte1 #x07) 18)left is the default in fxarithmetic-shift
@ -62,3 +62,2 @@(defwriter-ext (write-delimited-string writer str)(writer.write-delimited (cut &BufferedWriter-write-string <> str)(defwriter-ext (write-delimited-string-utf8 writer str)this file is dead and i am removing it in my pr
@ -22,0 +21,4 @@(spawn/name 'echo-server-handler echo-server-handler cli)(lp)))(catch (e)(unless (io-closed-error? e)Closed?
@ -50,0 +53,4 @@(DatagramSocket-sendto sock (unbox peer) buffer 0 read)(lp))))(catch (e)(unless (io-closed-error? e)Closed?
@ -77,0 +97,4 @@;; Detach restores any unread buffered content to the port.(let (reader (open-input-port-buffered-reader i))(unwind-protect (f reader)(using (buf (interface-instance-object reader) : DetachableBuffer)don't, just use buffer-detach! from bio/buffer which avoids the cast and allocation.
@ -77,0 +105,4 @@((u8vector? i)(let (reader (open-buffered-reader i))(unwind-protect (f reader) (BufferedReader-close reader))))((or (eq? i #t) (eq? i 'stdin))that's just weird.
@ -77,0 +126,4 @@(unwind-protect (f writer) (BufferedWriter-flush writer))))((is-Writer? o)(let (writer (open-buffered-writer o))(unwind-protect (f writer) (BufferedWriter-flush writer))))buffer-detach!
@ -77,0 +139,4 @@(let (writer (open-buffered-writer #f))(f writer)(get-memory-output-u8vector writer)))((or (eq? o 'stdout) (eq? o #t))that's just weird.
@ -0,0 +223,4 @@(defstruct JSONRPCRequestHandler((processor : :procedure)log)what type is this naked thing?
we already have the log in the request, drop it?
@ -0,0 +235,4 @@;; NB: This will catch any exception raised and convert it into an error notified to the client.;; TODO: have an optional parameter to specify a logging facility for those errors we find.(def (json-rpc-handler (req : Request) (res : ResponseHandler)(processor : :procedure) (log :? :procedure := #f))we have an actual log that is in the request.
please drop this ad-hocery.
That was code from before there was such a log. Nice. Thanks.
@ -0,0 +247,4 @@((POST) (json-rpc-handler/POST req res processor log)) ;; preferred method((GET) (json-rpc-handler/GET req res processor log)) ;; mostly for testing(else(when log (log [json-rpc-handler: 'BAD-HTTP-METHOD http-method]))bad ad-hocery.
Use the request log and log.debug with it so that it doesn't have any computational effect unless debug level is set.
you can even log.info or log.warn it.
@ -0,0 +283,4 @@json(catch (_)(json-rpc-handler/responsereq res log 'BAD-GET (hash ("jsonrpc" json-rpc-version) ("error" (parser-error))))you should probably also log the exception.
@ -0,0 +326,4 @@(try(json->u8vector response-json)(catch (_)(when log (log [json-rpc-handler: request-json 'BAD-JSON-RESPONSE]))use the request log.
OK, but there should be a display-exception-to-string, or something like write-exception that has an implicit with-buffered-writer.
there is exception->string
@ -0,0 +318,4 @@(def (json-rpc-handler/response(req : Request)(res : ResponseHandler)logdrop
@ -0,0 +178,4 @@decimal-mark: (decimal-mark #\.)group-separator: (group-separator #f)exponent-allowed: (exponent-allowed #f))(with-buffered-reader (reader)is this a procedure call?
if no drop the parenthesis in the macro.
if yes, add a contract.
this is just confusing,
@ -0,0 +412,4 @@always-sign?: (always-sign? #f)decimal-mark: (decimal-mark #\.)precision-loss-behavior: (precision-loss-behavior 'error))(with-buffered-writer (writer)same confusion here, is this a procedure call?
@ -0,0 +31,4 @@;; An extensible vector containing the increasing sequence of all small enough primes;; NB: the initial 0 is so the useful array indices start with 1, keeping with convention.;; Note that this ExtensibleVector must contain all the primes already in the prime-sieve below.(def primes (list->ExtensibleVector '(0 2 3 5 7 11 13)))why so few? is there a specific use case? we can definitely go up more, maybe the first 100 primes.
As the name implies, the ExtensibleVector gets automatically extended if and when you actually start using the sieve.
@ -0,0 +86,4 @@;; return true if the sieve found the number to be prime,;; false if the number was found to be composite,;; raise an error if the sieve wasn’t run far enough to tellwhat weird ass character is this?
@ -0,0 +27,4 @@(if width (fxmax content width) content)));; Write exact integer n in the given base to writer.;; base: radix 2–36; lowercase letters for digits above 9 (unless upper-case?).?
@ -0,0 +35,4 @@;; pad: ASCII fill char for width padding (default #\space; non-ASCII is an error).;; #\0 with align: 'right puts sign before zeros, any other char puts pad before sign.;; align: 'right (default), 'left, or 'center.;; upper-case?: #t = use uppercase A–F for hex digits.?
@ -17,3 +16,1 @@(to-string(lambda (self)(format "%a" self))))(to-string object->string))not the same, especially with objects.
revert dr sloppenheimer.
I was somehow experiencing a circular dependency with the format...
We should display a circular path when that happens.
I'll ask my slop machine to print that path.
@ -0,0 +108,4 @@(def (atomic-counter (initial-value : :fixnum := 0))(let (v (vector initial-value))(lambda ((increment : :fixnum := 1))this contract is going to be checked every fucking time at runtime.
we need a specialization for 1 or make a macro that avoids this.
@ -0,0 +16,4 @@(import(for-syntax :std/number/misc):std/error:std/io/apidrop the /api, same thing dr sloppenheimer.
@ -0,0 +351,4 @@(defsyntax (defintprocs stx)(def (make-def int size &int-ref &int-set!)(with-syntax ((size size)(&int-ref/be (stx-identifier &int-ref &int-ref "/be"))with-identifiers to save all those stx-identifier
4d3022b7a2to9e59dcdf39@ -433,2 +436,4 @@(def (call-with-getopt-parse gopt hash fun)(apply fun (getopt-parse->function-arguments gopt hash)))(def (call-with-processed-command-line processor command-line function)why is this here?
you should probably have it in multicall.
@ -435,0 +443,4 @@((list? processor)(call-with-processed-command-line (apply getopt processor) command-line function))))(def (->getopt-spec spec)what is this? where is it used?
this doesn't necessarily create a getopt spec.
I think it shouldn't be in this module, and i doubt it should exist at all.
@ -13,0 +63,4 @@;; string keys, walist result(def (read-json-walist/string (reader : BufferedReader) (opt : JSONReadOptions))(let* ((seen (make-hash-table size: 8))why 8? better just dont specify any size?
@ -13,0 +87,4 @@;; symbol keys, walist result(def (read-json-walist/symbol (reader : BufferedReader) (opt : JSONReadOptions))(let* ((seen (make-hash-table test: eq?))make-hash-table-eq
@ -13,0 +111,4 @@;; symbol keys, hash-table result(def (read-json-hash/symbol (reader : BufferedReader) (opt : JSONReadOptions))(let* ((obj (make-hash-table test: eq?))make-hash-table-eq
@ -13,0 +134,4 @@;; string keys, hash-table result(def (read-json-hash/string (reader : BufferedReader) (opt : JSONReadOptions))(let* ((obj (make-hash-table size: 8))again, wtf is this 8. don't specify any.
@ -0,0 +97,4 @@(walist (map (match <> ([name . index] (cons name (##structure-ref x index type name))))(class-type-printable-slots type)))))(def (trivial-json->object strukt json (defaults_ #f))this is pretty horrible slop. I am not sure i want it in the codebase.
also the name trivial is just misleading, drop it.
not also drop it, write an elegant version. this is shit unreadable code.
@ -0,0 +99,4 @@(def (trivial-json->object strukt json (defaults_ #f))(def defaults (or defaults_ (hash)))(def offsets (cast HashTable::interface (class-type-slot-table strukt)))just (HashTable blah) stupid clanker.
@ -0,0 +110,4 @@(else #f)))(or (hash-get offsets slot)(raise-bad-argument trivial-json->object "json key for struct" [key strukt json])))(def object (make-object strukt n))just new-instance
@ -0,0 +136,4 @@(raise-bad-argument trivial-json->object "unbound fields" [unbounds strukt json (hash->list defaults)]))object)(defmethod {:json :object}this has to become an interface.
@ -158,0 +209,4 @@(write-json-vector/pretty writer obj ctx)(write-json-vector writer obj ctx)))((hash-table? obj)(let (lst (hash->list obj))wait, we can avoid this allocation in many cases no?
@ -37,3 +38,3 @@(fx+ start remaining)))(read (__bio-input-buffer-read delim.input output start end need)))(set! delim.remaining (- remaining read))(set! delim.remaining (fx- remaining read))remaining is definitely fixnum?
otherwise bug.
@ -234,0 +608,4 @@(OutputBuffer(string-port-output-buffer(drain!(lambda (self buf whi)no need for the lambda, just __string-port-drain!
@ -560,3 +562,1 @@([](finish (cons next chars) separators-count)))))(finish chars separators-count))))))(if (null? separators-rest)why is this better code than previously. fucking slop/.
the clanker is enraging.
@ -0,0 +38,4 @@:std/net/http/server/request:std/net/http/server/status(only-in :std/error raise-bad-argument deferror-class IOError Error raise/context)(only-in :std/sync/atom atomic-counter)drop this only-in cargo cult
@ -0,0 +106,4 @@(def (atom-increment! atom (increment 1))(swap! atom + increment))(def (atomic-counter (initial-value : :fixnum := 0))this is a total fail.
@ -0,0 +115,4 @@(if (##fx= (##vector-cas! v 0 newval oldval) oldval)oldval(retry)))))(case-lambdathis is a pretty terrible use of case-lambda.
you will marshall the arguments in a list in every call and do dynamic dispatch.
this is so inefficient i would never use this counter utility.
i think that it needs to be redesigned to not use a closure at all. Have a counter object and a procedure that modifies and then we can have optional args and whatnot as the compiler can inline the direct calls.
46404d9aa0tod260693212d260693212tocc15941c8acc15941c8atoa5bf6ca4f2very few things left to complain.
@ -89,1 +74,3 @@result))))(cons(with-syntax ((writer writer) (arg arg) (flags flags)(width width) (precision precision)indent
@ -90,0 +82,4 @@(['float arg how flags width precision](loop rest(cons (with-syntax ((writer writer) (arg arg) (flags flags)(width width) (precision precision)indent.
@ -560,3 +564,1 @@([](finish (cons next chars) separators-count)))))(finish chars separators-count))))))(if (null? separators-rest)pleased deslopify this to what it was before.
@ -0,0 +139,4 @@(exact-integer? n))(definline (sint8? (n : :t)) => :boolean(and (fixnum? n) (<= -128 n 127)))fx<=
@ -0,0 +142,4 @@(and (fixnum? n) (<= -128 n 127)))(definline (sint16? (n : :t)) => :boolean(and (fixnum? n) (<= -32768 n 32767)))fx<=
b7fddc2d55to2ef5e62b02