diff --git a/docs/BUILT.md b/docs/BUILT.md index e4b68ff..70f538a 100644 --- a/docs/BUILT.md +++ b/docs/BUILT.md @@ -34,9 +34,12 @@ type and logical on an unsigned one), and `rand-f32`. only a regression test if the sequence is byte-identical on native and wasm32 (plan.org, RNG is ours). `rand-seed` sets the state. This is what the bitwise operators were added for. -**Enums and keywords.** `(defenum Name [member value ...])` gives a type that is an `i32` at run time and its own type +**Enums and keywords.** `(defenum Name [member value? ...])` gives a type that is an `i32` at run time and its own type in the checker, so `:space` at a call site resolves against the parameter's enum and a typo is an error there rather than a wrong number later. A keyword means nothing where no enum is expected — there is no keyword type to fall back on. +A member's value may be left out, and then it is the one above it plus one, starting at 0 — C's rule, because these +enums are as often a transcription of a header as they are original. A value written twice is an alias and is allowed; +a value autoincrement *walks into* is refused, naming both members, because nothing in the source chose it. ## Why the FFI goes through a C shim diff --git a/lib/parse.ml b/lib/parse.ml index e2518e7..bdfb134 100644 --- a/lib/parse.ml +++ b/lib/parse.ml @@ -924,19 +924,82 @@ let rec decl (f : Form.t) : Ast.decl = | _ -> fail f "%s" usage) | _ -> fail f "%s" usage) + (* A member's value is optional and autoincrements, which is C's rule and is + here for C's reason: the enums this language writes are as often a + transcription of a header as they are original, and every value written by + hand is a value that can be wrong. [0 1 2 3 4] typed out is fine until a + member is inserted in the middle, and then the renumbering is a manual + edit of every line below it. + + Which values were *written* does not survive this form. [Ast.Defenum] + holds resolved numbers, so by the time [Check] sees an enum the difference + between a number someone chose and one autoincrement produced is gone -- + and there are no per-member locations in the AST to point at either. That + is why the collision rule below is enforced here and not in the checker + beside the duplicate-*name* rule: this one is a question about the source + text, and the parser is the last pass that can still answer it. *) | List ({ v = Sym "defenum"; _ } :: args) -> (match args with | [ n; { v = Form.Vec ms; _ } ] -> - let rec pairs = function + (* Each member becomes its name, its value, whether that value was + written, and where the name is. The last two exist only so the + refusal below can be made; neither reaches the AST. *) + let rec members next = function | [] -> [] - | { v = Form.Sym m; _ } :: { v = Form.Int k; _ } :: rest -> - (m, k) :: pairs rest + | { v = Form.Sym m; loc } :: { v = Form.Int k; _ } :: rest -> + (m, k, true, loc) :: members (Int64.add k 1L) rest + | { v = Form.Sym m; loc } :: rest -> + (m, next, false, loc) :: members (Int64.add next 1L) rest | bad :: _ -> - fail bad "an enum member is a name followed by an integer, found %s" - (Form.to_string bad) + fail bad + "an enum member is a name, optionally followed by an integer, \ + found %s" (Form.to_string bad) in - mk (Ast.Defenum (sym n, pairs ms)) - | _ -> fail f "defenum is (defenum Name [member value ...])") + let ms = members 0L ms in + (* A duplicate value that was written is an alias and is meant: a [Count] + or a [Last] pointing at a value another member already holds is how C + spells the end of a range, and refusing it would refuse a real idiom. + A duplicate that autoincrement walked into is nobody's decision. It + happens when a member above is renumbered or one is inserted, and the + result is two names for one number with nothing in the source saying + so -- silently, and the program still compiles, and one of the two is + now unreachable through a [match] on the other. That silence is the + same failure class as a silent misparse, so the implicit member is + refused; writing its value out is both the fix and the way to say the + alias was intended. + + Every member is resolved before any of this runs, because the member + an autoincrement collides with is as often below it as above: in + [(defenum E [A B 0])] it is [A], the implicit one, that has to be + refused, and a left-to-right check would never see [B] coming. *) + let indexed = + List.mapi (fun i (m, v, explicit, loc) -> (i, m, v, explicit, loc)) ms + in + List.iter + (fun (i, m, v, explicit, loc) -> + if not explicit then + match + List.find_opt + (fun (j, _, ov, _, _) -> j <> i && Int64.equal ov v) + indexed + with + | None -> () + | Some (_, other, _, _, oloc) -> + Loc.failk "parse/enum-autoincrement-collision" loc + ~notes: + [ Loc.note oloc + (Printf.sprintf "%s has the value %Ld" other v) ] + "%s has no value of its own, so it autoincrements to %Ld, \ + which is the value %s already has. Give %s its value \ + explicitly if the two are meant to be one number under two \ + names, or a value no other member holds" + m v other m) + indexed; + mk (Ast.Defenum (sym n, List.map (fun (m, v, _, _) -> (m, v)) ms)) + | _ -> + fail f + "defenum is (defenum Name [member value? ...]). A member with no \ + value takes the previous member's plus one, and the first takes 0") | List ({ v = Sym "defvar"; _ } :: args) -> (match args with diff --git a/test/test_flan.ml b/test/test_flan.ml index ce15c6c..b5c9524 100644 --- a/test/test_flan.ml +++ b/test/test_flan.ml @@ -523,6 +523,61 @@ let () = parse_rejects "splice not inside a bracket" "(defn f [] Form `~@xs)" ~needle:"nothing here for it to splice into"; + (* ── defenum: a value is optional, and autoincrements ──────────── *) + (* The numbers are the whole of what the form means, so they are what is + asserted on -- the parser has resolved them by the time a decl exists, and + nothing downstream can tell an implicit value from a written one. *) + let enum_values name src want = + match (parse_decl src).d with + | Defenum (_, ms) -> + let show vs = String.concat " " (List.map Int64.to_string vs) in + let got = List.map snd ms in + if got <> want then begin + incr failures; + Printf.printf "FAIL %s\n wanted: [%s]\n got: [%s]\n" + name (show want) (show got) + end + | _ -> check (name ^ ": parses as a defenum") false + in + enum_values "every member implicit" "(defenum E [A B C])" [ 0L; 1L; 2L ]; + enum_values "every member explicit" "(defenum E [A 3 B 9 C -1])" + [ 3L; 9L; -1L ]; + (* The mixed case is the point of the feature: an explicit value resets the + count, and the members below it carry on from there. *) + enum_values "an implicit member follows the explicit one above it" + "(defenum E [A B C 10 D])" [ 0L; 1L; 10L; 11L ]; + (* The C idiom the explicit-duplicate rule exists for. *) + enum_values "a written duplicate is an alias and is kept" + "(defenum E [First 0 Second 1 Last 1])" [ 0L; 1L; 1L ]; + enum_values "an enum with no members at all" "(defenum E [])" []; + + (* A value autoincrement walked into is refused, because nothing in the + source chose it -- and the refusal has to name the *other* member, which + is the half a reader cannot see from the line that failed. Three needles + for one source: the message is only doing its job if both names, the + number, and the way out are all in it. *) + parse_rejects "an autoincrement onto a value already taken" + "(defenum E [A 0 B 1 C 0 D])" + ~needle:"D has no value of its own, so it autoincrements to 1"; + parse_rejects "the refusal names the member already holding the value" + "(defenum E [A 0 B 1 C 0 D])" + ~needle:"which is the value B already has"; + parse_rejects "the refusal says how to say the alias was meant" + "(defenum E [A 0 B 1 C 0 D])" + ~needle:"Give D its value explicitly"; + (* The member collided with is as often below as above: here it is [A], the + implicit one, that is refused, and a left-to-right check would pass it. *) + parse_rejects "an autoincrement onto a value written further down" + "(defenum E [A B 0])" + ~needle:"A has no value of its own, so it autoincrements to 0"; + + (* Genuinely malformed input still says what a member is, in the grammar the + form now has. *) + parse_rejects "an enum member that is not a name" "(defenum E [1 A])" + ~needle:"an enum member is a name, optionally followed by an integer"; + parse_rejects "a defenum with no member vector" "(defenum E)" + ~needle:"defenum is (defenum Name [member value? ...])"; + (* ── Malformed syntax is caught with a location ────────────────── *) parse_rejects "odd let bindings" "(let [a])"; parse_rejects "odd field pairs" "(defstruct S [a])";