Two rules the checker was missing

A shift by the operand's own width or more is poison in LLVM, not a wrong
number: (<< 1 32) at -O2 compiled to a bare retq. A literal count out of range
is now rejected in check.ml, and emit.ml masks a computed one to width - 1,
which is what the hardware does and which LLVM folds away for a constant.

There is one top-level namespace, but the environment's tables are per-kind, so
only a function was ever checked for a duplicate. (defn item ...) beside
(defvar item ...) type checked and then died in LLVM as a redefinition of
'@flan.item'; two colliding type declarations were not caught anywhere. One
pass over Ast.declared_name now runs before every other collection pass. That
function lives in ast.ml because Load needs the same set - the names an import
renames - and two copies would drift.

NEXT.md

@@ -237,13 +237,38 @@ Non-local exit is lowered explicitly: `return`, `some` and a failed bounds
 check are branches, never platform unwinding, so wasm32 needs no exception
 proposal.
 
-## Sharp edges found and left visible
+## Sharp edges
+
+Most of these are edges the language keeps and you should know about. Two —
+the top-level namespace and the shift count, both found by review after
+milestone 4 — were bugs that reached LLVM or ran wrong, and are **fixed**; each
+says so. They stay written down because each one is now a rule the checker
+enforces, and a later change could quietly drop it.
 
 - **An index converts from a narrower integer and never from a wider one.**
   `(nth colors current-color)` with a `u32` index works — anything above 2³¹
   truncates to a negative `i32` and the unsigned bounds check rejects it. An
   `i64` index is refused with the reason: 2³²+5 truncates to 5 and would read
   the wrong element with no trap at all.
+- **There is one top-level namespace, and `check.ml` now enforces it.** The
+  environment's tables are per-kind — structs, unions, aliases, enums,
+  functions, externs and globals each have their own — so only a function was
+  ever checked for a duplicate. `(defn item …)` beside `(defvar item …)` type
+  checked and then died in LLVM as `redefinition of function '@flan.item'`, a
+  message about an emitted symbol with no source location left, and two
+  colliding *type* declarations were not caught anywhere. One pass over
+  `Ast.declared_name` now runs before every other collection pass and rejects
+  the second declaration of a name whatever kind either one is. `declared_name`
+  lives in `ast.ml` because `Load` needs exactly the same set — the names an
+  import renames — and two copies of that list would drift.
+- **A shift count is bounded, two different ways.** A shift by the operand's
+  own width or more is *poison* in LLVM, not a wrong number: `(defn main [] i32
+  (<< 1 32))` compiled at -O2 to a bare `retq`, returning an undefined value. A
+  literal count out of range is now rejected in `check.ml` — that is the typo
+  case — and `emit.ml` masks a computed count to `width - 1`, which is what the
+  hardware does anyway and which LLVM folds away whenever the count is
+  constant. The prelude's rotate masks its own count; that is now redundant but
+  harmless.
 - **A `u64` literal is its 64-bit pattern**, so `0xcbf29ce484222325` is a real
   `u64` and not an error. The cost is that a negative *decimal* literal is
   accepted as a `u64` too, because the reader records the value and not how it

lib/ast.ml

@@ -108,3 +108,14 @@ and decl_kind =
 and variant = { vname : string; vfields : field list; vloc : Loc.t }
 
 and init = Zeroed | Uninit | Init of expr
+
+(* Every top-level name a declaration introduces, whatever kind it is. There is
+   one top-level namespace, so this is both the set [Load] renames on an import
+   and the set [Check] refuses to see twice — one definition, so the two cannot
+   drift apart. *)
+let declared_name (d : decl) =
+  match d.d with
+  | Defenum (n, _) | Defalias (n, _) | Defstruct (n, _) | Defunion (n, _)
+  | Defvar (n, _, _) | Defconst (n, _, _) -> Some n
+  | Declare (fn, _) | Defn fn -> Some fn.name
+  | Package _ | Import _ -> None

lib/check.ml

@@ -767,6 +767,19 @@ and named_call ctx ~want loc name args =
      | Types.Int _ -> ()
      | other -> fail loc "%s takes integers, found %s" name
                   (Types.to_string other));
+    (* A shift by the operand's own width or more is poison in LLVM, which at
+       -O2 turns the whole function into an undefined value rather than into a
+       wrong number. A literal count is rejected here — that is the typo — and
+       [emit] masks a computed one, so no shift can reach the hardware out of
+       range. *)
+    (match a.Tast.ty, b.Tast.e with
+     | Types.Int k, Tast.Int (n, _) when p = Tast.Shl || p = Tast.Shr ->
+       let w = Int64.of_int (Types.bits k) in
+       if Int64.unsigned_compare n w >= 0 then
+         fail loc
+           "%s by %Ld is out of range for %s, which is %d bits wide" name n
+           (Types.to_string a.Tast.ty) (Types.bits k)
+     | _ -> ());
     prim p a.Tast.ty [ a; b ]
   (* (min a b) and (max a b) evaluate each operand once — hence the slots —
      because a min over two calls must not call either of them twice. *)
@@ -983,6 +996,22 @@ let rec const_int env (e : Ast.expr) : int64 option =
   | _ -> None
 
 let collect env (decls : Ast.decl list) =
+  (* One pass over every declaration kind before any of the others, because
+     the tables below are per-kind — structs, unions, aliases, enums, functions
+     and globals each have their own — and a collision between two of them
+     would otherwise be found by LLVM, as [redefinition of function
+     '@flan.item'], or not at all. A [defn item] and a [defvar item] are two
+     declarations of one name and are rejected here. *)
+  let claimed = Hashtbl.create 64 in
+  List.iter
+    (fun (d : Ast.decl) ->
+       match Ast.declared_name d with
+       | None -> ()
+       | Some n ->
+         if Hashtbl.mem claimed n then
+           fail d.Ast.dloc "%s is defined twice" n;
+         Hashtbl.add claimed n ())
+    decls;
   (* Names first, so a struct may mention one declared below it. *)
   List.iter
     (fun (d : Ast.decl) ->
@@ -1086,8 +1115,6 @@ let collect env (decls : Ast.decl list) =
                { Tast.vname = v.Ast.vname;
                  vfields = List.map field v.Ast.vfields }) vs }
        | Ast.Defn fn ->
-         if Hashtbl.mem env.fns fn.Ast.name then
-           fail loc "%s is defined twice" fn.Ast.name;
          let params =
            List.map (fun (p : Ast.field) -> resolve env p.Ast.fty) fn.Ast.params
          in

lib/emit.ml

@@ -555,6 +555,20 @@ and prim f (e : Tast.expr) (p : Tast.prim) (args : Tast.expr list) =
       | Types.Int k, _ -> if Types.signed k then "ashr" else "lshr"
       | t, _ -> failwith ("bitwise on " ^ Types.to_string t)
     in
+    (* The count is masked to the operand's width. LLVM makes an over-wide
+       shift poison, and a poison return at -O2 is a function that returns
+       nothing at all; masking is what the hardware does anyway, and LLVM folds
+       the [and] away whenever the count is a constant. [check] has already
+       rejected a literal that is out of range, so this only ever fires on a
+       computed count. *)
+    let b =
+      match x.Tast.ty, p with
+      | Types.Int k, (Tast.Shl | Tast.Shr) ->
+        let m = fresh f in
+        ins f "%s = and %s %s, %d" m (ll x.Tast.ty) b (Types.bits k - 1);
+        m
+      | _ -> b
+    in
     let t = fresh f in
     ins f "%s = %s %s %s, %s" t op (ll x.Tast.ty) a b;
     t

lib/load.ml

@@ -228,15 +228,7 @@ let qualify_decl owned alias (d : Ast.decl) : Ast.decl =
 (* Every top-level name the package declares — types and values alike, since a
    use site is rewritten by name and the two never collide in one namespace. *)
 let owned_names (ds : Ast.decl list) =
-  List.filter_map
+  List.filter_map Ast.declared_name ds
-    (fun (d : Ast.decl) ->
-       match d.Ast.d with
-       | Ast.Defenum (n, _) | Ast.Defalias (n, _) | Ast.Defstruct (n, _)
-       | Ast.Defunion (n, _) | Ast.Defvar (n, _, _) | Ast.Defconst (n, _, _) ->
-         Some n
-       | Ast.Declare (fn, _) | Ast.Defn fn -> Some fn.Ast.name
-       | Ast.Package _ | Ast.Import _ -> None)
-    ds
 
 let import ~loc alias dir =
   let files = entries dir ".flan" in

test/test_flan.ml

@@ -552,6 +552,30 @@ let () =
   (* A folded constant skips [check], so its range check has to be its own. *)
   rejects_check "a folded constant is still range-checked"
     "(defconst c u8 300) (defn f [] u8 c)" ~needle:"does not fit in u8";
+  (* One top-level namespace, enforced across declaration kinds. Each of these
+     used to pass the checker — the tables are per-kind — and be caught by LLVM
+     as a redefinition of an emitted symbol, or not caught at all. *)
+  rejects_check "a global defined twice"
+    "(defvar x i32 1) (defvar x i32 2)" ~needle:"defined twice";
+  rejects_check "a constant shadowing a variable"
+    "(defconst c 1) (defvar c i32 2)" ~needle:"defined twice";
+  rejects_check "a function and a global"
+    "(defn item [] i32 1) (defvar item i32 2)" ~needle:"defined twice";
+  rejects_check "a struct and an alias"
+    "(defstruct P [x i32]) (defalias P i32)" ~needle:"defined twice";
+  rejects_check "an enum and a struct"
+    "(defenum E [a 1]) (defstruct E [x i32])" ~needle:"defined twice";
+  rejects_check "an extern and a constant"
+    "(declare cw [] \"flan_cw\") (defconst cw 1)" ~needle:"defined twice";
+  (* A shift by the operand's own width or more is poison in LLVM, and at -O2
+     a poison return is a function that returns nothing at all. A literal count
+     is rejected; a computed one is masked in [emit]. *)
+  rejects_check "a shift past the operand's width"
+    "(defn f [] i32 (<< 1 32))" ~needle:"out of range";
+  rejects_check "a right shift past the operand's width"
+    "(defn f [] u8 (>> (u8 1) 8))" ~needle:"out of range";
+  accepts "a shift by the widest count in range"
+    "(defn f [] i32 (<< 1 31))";
   (* An index converts from a narrower integer and never from a wider one. *)
   accepts "a u32 index" "(defvar a [4 u32]) (defn f [] u32 (let [i 2] (at a (u32 i))))";
   rejects_check "an i64 index"