A defn named after a builtin now wins, and says so once
The author's rule: "allow shadowing but warn". A user (defn get ...) is legal, the user's definition wins at every call site in the file that wrote it, and the compiler warns once at the definition. Builtin-wins was never a rule anybody wrote: named_call is one match on the name, the builtin arms are string literals, and the three arms that look a name up are the last three in it. So a guard goes first, the trailing three are factored into ordinary_call, and both routes into it resolve a name the same way. The shadow stops at the file that declared it. An imported package's names were qualified at the import, so a get written inside one is the builtin's and stays the builtin's; the prelude is excluded by its file for the same reason. programs/shadow-builtin.flan is both halves at once. The warning prints from build_program, which is what every command and the dev daemon's reload go through, in the shape --warn-memory established: file:line:col, the squiggle, and an exit status that does not move. And the message that described the old world is gone — the builtin-arity note said a defn does not replace a builtin, which is no longer true and is no longer reachable.
This commit is contained in:
parent
b87ae11fa8
commit
e03028819c
129
FIX.org
129
FIX.org
@ -2042,6 +2042,11 @@ wins every call and the definition is still unreachable; what changed is that
|
||||
the arity message says so and notes the definition. Refusing the shadowing is
|
||||
a language decision and was left to the author.
|
||||
|
||||
[Superseded the same day by the author's decision — see "Shadowing a builtin"
|
||||
below. The builtin no longer wins, the definition is no longer unreachable,
|
||||
and the arity note this paragraph describes has been removed along with the
|
||||
world it described.]
|
||||
|
||||
* ~int~ and ~float~ as builtin aliases, 2026-09-20
|
||||
|
||||
The author, on the foreign-spelling list the diagnostics pass had just
|
||||
@ -2098,8 +2103,12 @@ was a builtin. The rule is decided by the target, at registration:
|
||||
- Anything else — refused: "int is a builtin alias for i32 and cannot be
|
||||
redefined as i64 — delete this defalias, or give the type another name".
|
||||
|
||||
The alternative was the ~arity~ precedent, where the builtin wins and a note
|
||||
surfaces at the error the shadowing caused. It does not transfer: a
|
||||
The alternative was the ~arity~ precedent, where the builtin won and a note
|
||||
surfaced at the error the shadowing caused — a precedent deleted later the
|
||||
same day, when shadowing a builtin became legal and the user's definition
|
||||
started winning instead (see "Shadowing a builtin" below); the reasoning
|
||||
below stands either way, because neither world has anywhere to put the note.
|
||||
It does not transfer: a
|
||||
~(defalias int i64)~ has no later error site to hang a note on. ~resolve_name~
|
||||
reaches ~ikind_of_name~ before the alias table, so the declaration would be
|
||||
read as ~i32~ at every use and nothing would ever say so. Silence was the one
|
||||
@ -2442,3 +2451,119 @@ failures were that row and the sixth was ~dev-trap-free-all~, so what is racy
|
||||
is ~trap_park~ itself and every row that calls it — which is exactly what the
|
||||
mechanism described there predicts. Per the sweep policy the ~@x86~ and
|
||||
~@sanitize~ sweeps were not run here.
|
||||
* Shadowing a builtin, 2026-09-20
|
||||
|
||||
The author's decision, in the author's words:
|
||||
|
||||
#+begin_quote
|
||||
"allow shadowing but warn" — a user ~(defn get ...)~ colliding with a builtin
|
||||
is legal, the USER'S definition wins at call sites (real shadowing, Clojure's
|
||||
model: the def takes over, a warning says so), and the compiler warns once at
|
||||
the definition site.
|
||||
#+end_quote
|
||||
|
||||
** Where builtin-wins actually lived
|
||||
Not in a table and not in a precedence list. ~named_call~ is one
|
||||
~match name with~ whose arms are the builtin names written out as string
|
||||
literals, and the three arms that look anything up — a local of ~Fn~ type,
|
||||
~gsigs~, then ~env.fns~ — are the last three in that match. So a builtin won
|
||||
because OCaml tried its arm first, and for no other reason. ~env.fns~ never
|
||||
outranked anything; it was simply never reached for a name spelled like a
|
||||
builtin. The old comment above ~arity~ said this outright ("the dispatch
|
||||
above reaches every builtin arm before it ever looks in [fns]") and is the
|
||||
only place it was written down.
|
||||
|
||||
** The resolution change
|
||||
One guard, first arm of ~named_call~:
|
||||
|
||||
: | _ when shadows_builtin ctx loc name -> ordinary_call ctx ~want loc name args
|
||||
|
||||
and the three trailing arms factored into ~ordinary_call~ so that both routes
|
||||
— falling past every builtin, and being sent straight there by the guard —
|
||||
resolve a name by exactly the same rules. Order is now total and reads the
|
||||
way a reader would guess: local of function type, then generic signature,
|
||||
then the function table, then the builtins, then the struct and the
|
||||
did-you-mean refusals.
|
||||
|
||||
~shadows_builtin~ asks three questions, cheapest first: is the name defined
|
||||
(the two tables and the scope), is it a builtin's (otherwise there is no
|
||||
order to change, and the enum/type-variable/machine-type cast arms keep
|
||||
theirs), and is the definition visible here.
|
||||
|
||||
** The warning, verbatim
|
||||
: shadow-builtin.flan:20:7: warning: get shadows the builtin get — every call in this program now reaches your definition
|
||||
: 20 | (defn get [p P] i32 (.x p))
|
||||
: | ~~~
|
||||
|
||||
Rendered by ~Loc.entry ~mark:'~' ~label:"warning: "~, which is the
|
||||
~--warn-memory~ precedent, so flycheck parses it exactly as it parses an
|
||||
error. Nothing raises and the exit status does not move. Unlike
|
||||
~--warn-memory~ it is behind no flag: there is nothing to tune, and the line
|
||||
is one line and rare.
|
||||
|
||||
It is printed from ~Check.build_program~ rather than from ~bin/main.ml~
|
||||
beside ~print_memory_warnings~, because every route into the compiler passes
|
||||
through that function — build, check, run, and the dev daemon's reload, which
|
||||
is where a defn is most likely to be written. The list itself is
|
||||
~Check.shadowed_builtins~, a pure function over the declarations, which is
|
||||
what the tests ask.
|
||||
|
||||
** Scope, settled from the code
|
||||
*Package-wide or program-wide: neither, and the mechanism already decided
|
||||
it.* ~Load~ qualifies every name an imported package declares to ~alias/name~,
|
||||
including its own uses of them, so a package's ~get~ is ~rl/get~ and cannot
|
||||
collide with a builtin at all. What is left is the other direction: a program
|
||||
that defines ~get~ and imports a package whose body calls the builtin ~get~.
|
||||
That call must keep meaning the builtin, and it does — ~shadows_builtin~
|
||||
answers false when the enclosing function's name is qualified, which inside
|
||||
an imported package is always. The prelude is excluded the same way, by its
|
||||
file: it is the language's own source and means the builtin wherever it
|
||||
writes one. ~programs/shadow-builtin.flan~ is both halves in one program: 7
|
||||
is the program's own one-argument ~(get p)~, 4 is the builtin ~get~ called
|
||||
inside the package it imports.
|
||||
|
||||
*Prelude macros.* No rule was needed: the namespace is already one.
|
||||
~(defn comment [x i32] i32 ...)~ against the prelude's ~(defmacro comment
|
||||
...)~ is refused today as "comment is defined twice", with a note at the
|
||||
prelude's definition, and the same for ~inc~ and ~dec~. Shadowing a builtin
|
||||
is a different question precisely because a builtin is not a declaration —
|
||||
it is an arm in the compiler, with nothing for a redefinition check to point
|
||||
at. Macros expand before checking and key on the head name unconditionally,
|
||||
so if the redefinition check were ever relaxed the macro would win and the
|
||||
defn would be unreachable; that is not a state this compiler can reach, and
|
||||
nothing was written to handle it.
|
||||
|
||||
*The one place the rule is conservative.* A bare REPL expression evaluated
|
||||
with no file behind it (origin ~<eval>~) is checked with an unqualified
|
||||
owner and so does shadow, which is right; a call written inside an imported
|
||||
package's *global initialiser* — no enclosing function, so no qualified owner
|
||||
— would not be excluded. Nothing in the corpus does that, and the fix if it
|
||||
ever matters is to carry the package's file rather than the owner's name.
|
||||
|
||||
** Pins
|
||||
- ~test_flan.ml~: the warning's kind, line and column; its message, matched
|
||||
whole and not by needle; that it carries no notes; that the source which
|
||||
used to be refused now checks; and that a program shadowing nothing warns
|
||||
not at all.
|
||||
- ~test_acceptance.ml~: ~programs/shadow-builtin.flan~ outputs ~7\n4\n~, and
|
||||
the ~@x86~ sweep compares both backends over the same file.
|
||||
- Removed: the ~check/builtin-arity~ kind, its message ("this is the builtin
|
||||
get, which a defn of the same name does not replace"), its note ("is also
|
||||
defined here, and this call is not reaching it — rename it to call it"),
|
||||
and the three checks that pinned them. The situation cannot arise: the call
|
||||
reaches the user's defn, whose arity is whatever it declared.
|
||||
- Changed: the builtin-arm/~Check.builtins~ cross-check reads ~named_call~'s
|
||||
source down to ~ | _ ->~ rather than ~ | _~, because the new first arm is
|
||||
guarded and stopping at it read the whole region as empty.
|
||||
|
||||
** One thing the new package cost
|
||||
A package under ~test/programs/pkgs/~ needs a ~glob_files~ line of its own in
|
||||
four places in ~test/dune~ — the test stanza and the ~@valgrind~, ~@x86~ and
|
||||
~@js~ sweeps — because dune's glob does not descend and the sweeps walk
|
||||
~programs/*.flan~ whole. Without it the corpus row fails with "no package
|
||||
at ..." and prints no FAIL line, only "1 failure(s)" at the end of the log:
|
||||
worth knowing, because a grep for FAIL says green over it.
|
||||
|
||||
** What was run
|
||||
~dune test --root .~ in the lane's worktree, forced: exit 0.
|
||||
The heavy sweeps (~@x86~, ~@sanitize~, ~@valgrind~) were left to the batch.
|
||||
|
||||
@ -49,7 +49,7 @@ runtime has only preformatted loc strings, no access to source text).
|
||||
| 4 | `unknown function prinltn` / `unknown name n` | check.ml:6259, 2732 | `(prinltn "hi")` | B | C | D | B | No did-you-mean, although `near_miss` (check.ml:670) is written, tested and wired — to **types only**. Point it at `env.fns`, `env.globals` and the local scope. Cheapest structural win on the list. |
|
||||
| 5 | `expected bool, found i32` | check.ml:1866 via `check_truthy` (3596) | `(let [x 1] (if x …))` | A | C | D | A | Caret is exactly right (the `check_truthy` loc work paid off). But the message never states Flan's truthiness rule — bool or dyn, nothing else — and never names the fix (`(not= x 0)`). Special-case the condition position. |
|
||||
| 6 | `binding 5 has no value — let takes name/value pairs` | parse.ml:670 | `(let [x i32 5] …)` | C | D | D | B | A type annotation in `let` is the single most natural thing for someone arriving from a typed language, and `let` has none. The message reads as if the user miscounted. Detect "middle form names a type" and say so: "`let` bindings take no type annotation — write `[x 5]`". |
|
||||
| 7 | `get takes 2 arguments, given 1` against the **user's own** `(defn get [p P] …)` | check.ml:5296 (builtin dispatch) + 6233 | `(defn get [p P] i32 …)` + `(get p)` | D | D | D | B | A user defn whose name collides with a builtin is silently shadowed, and then the arity refusal is measured against the *builtin*, pointing at the user's call. Either refuse the shadowing definition at its `dloc` with a note, or report the arity against the definition the user can see. |
|
||||
| 7 | `get takes 2 arguments, given 1` against the **user's own** `(defn get [p P] …)` | check.ml:5296 (builtin dispatch) + 6233 | `(defn get [p P] i32 …)` + `(get p)` | D | D | D | B | A user defn whose name collides with a builtin is silently shadowed, and then the arity refusal is measured against the *builtin*, pointing at the user's call. Either refuse the shadowing definition at its `dloc` with a note, or report the arity against the definition the user can see. **Settled 2026-09-20, neither way: the author chose "allow shadowing but warn" — the defn wins at every call site in its own file and the compiler warns once at the definition. See FIX.org, "Shadowing a builtin".** |
|
||||
| 8 | `unhandled Boom` | flan_rt.c:646 | `(defstruct Boom [why i32])` + `(error (Boom {.why 7}))`, `flan run` | D | C | D | B | Three words. No location (not even the `error` site, which the emitter knows), no field values, no list of the handlers that were in scope. The condition system is a headline feature and this is its failure mode. |
|
||||
| 9 | `the collection nosuch: is a directory named nosuch somewhere above /…/. , and there is none` | load.ml:120 | `(import zz "nosuch:thing")` | B | C | C | D | Reads as an assertion immediately contradicted. Also emits a bare `/.` on the path. Rewrite as a plain statement of the search ("no directory named `nosuch` between here and the root") and list what collections *were* found. |
|
||||
| 10 | `and`'s last operand gets the previous operand's caret | parse.ml `shortcircuit`, via check.ml:3632 | `(println (and true true (vec-new i32)))` | D | B | C | A | Already diagnosed in FIX.org:1036 with three rejected fixes; the accepted one — `check_if` preferring the arm that is not a compiler temp when choosing which to blame — is a check.ml change nobody owned. This pass owns check.ml. |
|
||||
|
||||
155
lib/check.ml
155
lib/check.ml
@ -4932,35 +4932,18 @@ and call_value ctx ~want loc (callee : Tast.expr) args =
|
||||
fail loc "this is a %s and not a function, so it cannot be called"
|
||||
(Types.to_string other)
|
||||
|
||||
(* A builtin's arity, and the one thing the caret cannot show: whether the
|
||||
count being measured against is the builtin's or a defn of the same name.
|
||||
A defn does not shadow a builtin — the dispatch above reaches every builtin
|
||||
arm before it ever looks in [fns] — so a user function called [get] is
|
||||
silently unreachable, and the refusal that followed measured the call
|
||||
against the builtin while pointing at a call the reader had written for
|
||||
their own. Said outright, with the definition alongside. *)
|
||||
and arity ctx loc name n args =
|
||||
if List.length args <> n then begin
|
||||
let notes =
|
||||
if Hashtbl.mem ctx.env.fns name then
|
||||
match Hashtbl.find_opt ctx.env.fn_locs name with
|
||||
| Some at ->
|
||||
[ Loc.note at
|
||||
(name ^ " is also defined here, and this call is not reaching \
|
||||
it — rename it to call it") ]
|
||||
| None -> []
|
||||
else []
|
||||
in
|
||||
let shadowed = notes <> [] in
|
||||
if shadowed then
|
||||
Loc.failk "check/builtin-arity" loc ~notes
|
||||
"%s takes %d argument%s, given %d — this is the builtin %s, which a \
|
||||
defn of the same name does not replace"
|
||||
name n (if n = 1 then "" else "s") (List.length args) name
|
||||
else
|
||||
fail loc "%s takes %d argument%s, given %d" name n
|
||||
(if n = 1 then "" else "s") (List.length args)
|
||||
end
|
||||
(* A builtin's arity. The count is the builtin's and can only be the
|
||||
builtin's: a defn of the same name written in the program now takes the
|
||||
call over before any builtin arm is reached ([shadows_builtin] at the top
|
||||
of [named_call]), so a call measured here is a call to the builtin and
|
||||
there is no second signature for the reader to have meant. The note that
|
||||
used to say otherwise — "this is the builtin get, which a defn of the same
|
||||
name does not replace" — described a resolution order this compiler no
|
||||
longer has. *)
|
||||
and arity _ctx loc name n args =
|
||||
if List.length args <> n then
|
||||
fail loc "%s takes %d argument%s, given %d" name n
|
||||
(if n = 1 then "" else "s") (List.length args)
|
||||
|
||||
(* The operators that fold: [+ - * /], [min]/[max] and the three bitwise
|
||||
combining operators all take two operands or more, and mean the same thing
|
||||
@ -5350,6 +5333,19 @@ and vec_at ctx loc (target : Tast.expr) (idx : Ast.expr list) =
|
||||
and named_call ctx ~want loc name args =
|
||||
let prim p ty args = expect ctx loc ~want (mk loc ty (Tast.Prim (p, args))) in
|
||||
match name with
|
||||
(* The user's own definition, first — before every builtin arm below.
|
||||
Clojure's rule: a [(defn get ...)] takes the name over, and a call
|
||||
written in the program that defines it reaches that definition rather
|
||||
than the builtin it is named after. The defn site is warned about once
|
||||
(see [shadowed_builtins]); the call sites say nothing, because at a call
|
||||
site there is nothing surprising left — the name means what the file
|
||||
says it means.
|
||||
|
||||
What this arm does NOT do is let one file's definition reach into
|
||||
another's: [shadows_builtin] answers false for a call in the prelude and
|
||||
for a call in imported package code, which is the same visibility rule a
|
||||
defn has everywhere else. *)
|
||||
| _ when shadows_builtin ctx loc name -> ordinary_call ctx ~want loc name args
|
||||
(* ── arithmetic and comparison ─────────────────────────────────── *)
|
||||
| "+" | "-" | "*" | "/" ->
|
||||
let p = match name with
|
||||
@ -7163,13 +7159,22 @@ and named_call ctx ~want loc name args =
|
||||
| _ -> prim (Tast.Cast target) target [ a ])
|
||||
|
||||
(* ── ordinary calls ────────────────────────────────────────────── *)
|
||||
| _ -> ordinary_call ctx ~want loc name args
|
||||
|
||||
(* Everything that is not a builtin arm: a local of function type, a generic
|
||||
signature, the function table, and the refusals for a name that is none of
|
||||
them. Reached two ways — by falling past every arm above, and by the
|
||||
shadowing guard at the very top of [named_call], which sends a call whose
|
||||
name the program has defined straight here. One function so that both
|
||||
routes resolve a name by exactly the same rules. *)
|
||||
and ordinary_call ctx ~want loc name args =
|
||||
match () with
|
||||
(* A local or a parameter holding a function value, called by the name it is
|
||||
bound to — which is what the body of [map] looks like. It is checked
|
||||
before the global function table and after every builtin: a binding
|
||||
shadows a defn of the same name (one namespace, ordinary lexical
|
||||
scoping), and nothing shadows [+]. A local of any *other* type falls
|
||||
through to the table, so a program that shadows a function name with an
|
||||
i32 and then calls the function still means the function. *)
|
||||
before the global function table: a binding shadows a defn of the same
|
||||
name (one namespace, ordinary lexical scoping). A local of any *other*
|
||||
type falls through to the table, so a program that shadows a function
|
||||
name with an i32 and then calls the function still means the function. *)
|
||||
| _ when (match lookup ctx name with
|
||||
| Some b -> (match b.bty with Types.Fn _ -> true | _ -> false)
|
||||
| None -> false) ->
|
||||
@ -7251,6 +7256,43 @@ and named_call ctx ~want loc name args =
|
||||
name name
|
||||
else Loc.failk "check/unknown-function" loc "unknown function %s" name
|
||||
|
||||
(* Does the program's own definition of this name take this call over?
|
||||
Three questions, and the order is the order that asks the fewest of them.
|
||||
|
||||
Is the name a builtin's at all. This is asked first and not second: it is
|
||||
false for every call to an ordinary function, which is most calls in most
|
||||
programs, and it is the question that stops the other two from running.
|
||||
Asking it also keeps the arms that are not calls — an enum cast, a cast to
|
||||
a type variable, a machine-type cast — exactly where they were, since a
|
||||
name that reaches one of those is not a builtin's either.
|
||||
|
||||
Is it defined — by the function table, by a generic signature, or by a
|
||||
local of function type. Two hash lookups and, only if both miss, the walk
|
||||
down the scope, which is a list.
|
||||
|
||||
And is the definition visible here. Shadowing follows a defn's visibility
|
||||
like anything else: the prelude is the language's own source and means the
|
||||
builtin wherever it writes one, and an imported package's names are
|
||||
qualified ([rl/get]) so a call written [get] inside one is the builtin
|
||||
too — recognised by the owner's name, which the import qualified along
|
||||
with everything else. Neither is shadowed by a definition in the file
|
||||
being compiled, which is the point: a package that defines [get] does not
|
||||
change what [get] means to its importer, and an importer that defines one
|
||||
does not change what it means inside the package. *)
|
||||
and shadows_builtin ctx loc name =
|
||||
let defined () =
|
||||
Hashtbl.mem ctx.env.fns name
|
||||
|| Hashtbl.mem ctx.env.gsigs name
|
||||
|| (match lookup ctx name with
|
||||
| Some b -> (match b.bty with Types.Fn _ -> true | _ -> false)
|
||||
| None -> false)
|
||||
in
|
||||
let visible () =
|
||||
not (String.equal loc.Loc.file Prelude.file)
|
||||
&& not (String.contains ctx.owner '/')
|
||||
in
|
||||
List.mem name !builtin_names && defined () && visible ()
|
||||
|
||||
(* ── A call to a generic function ───────────────────────────────────────
|
||||
The whole of instantiation, and it is at the call site because the call
|
||||
site is the only place the concrete types exist. Odin does the same thing
|
||||
@ -7809,6 +7851,38 @@ let builtins : (string * string * string) list =
|
||||
checked before this module is loaded. *)
|
||||
let () = builtin_names := List.map (fun (n, _, _) -> n) builtins
|
||||
|
||||
(* ── The one thing shadowing owes the reader ────────────────────────────
|
||||
A defn named after a builtin is legal and it wins ([shadows_builtin]), and
|
||||
that is a large thing to have happened in silence: every [(get m k)] in
|
||||
the file now means something the reader has to go and look at. So it is
|
||||
said once, where the decision was made, and never again at the call sites
|
||||
— a footgun notice, not a lint.
|
||||
|
||||
It is a warning and it says so in the one way that matters: nothing raises
|
||||
and the exit status does not move. Unlike [memory_sites] it is behind no
|
||||
flag, because there is nothing to tune — a program either renamed a
|
||||
builtin or it did not, and the line is one line and rare.
|
||||
|
||||
The prelude is skipped: its defns are the language's own and a collision
|
||||
there is a compiler bug rather than news for whoever is compiling. So are
|
||||
qualified names, for the reason [shadows_builtin]'s [visible] gives —
|
||||
[rl/get] is not [get] and shadows nothing. *)
|
||||
let shadowed_builtins (decls : Ast.decl list) : Loc.diag list =
|
||||
List.filter_map
|
||||
(fun (d : Ast.decl) ->
|
||||
match d.Ast.d with
|
||||
| Ast.Defn fn
|
||||
when List.mem fn.Ast.name !builtin_names
|
||||
&& not (String.contains fn.Ast.name '/')
|
||||
&& not (String.equal fn.Ast.nloc.Loc.file Prelude.file) ->
|
||||
Some
|
||||
(Loc.diag ~kind:"check/shadows-builtin" fn.Ast.nloc
|
||||
(Printf.sprintf
|
||||
"%s shadows the builtin %s — every call in this program now \
|
||||
reaches your definition" fn.Ast.name fn.Ast.name))
|
||||
| _ -> None)
|
||||
decls
|
||||
|
||||
(* ── Declarations: pass 1, collect ─────────────────────────────────── *)
|
||||
|
||||
(* Constant folding, only over integers and only for defconst — enough for an
|
||||
@ -9414,6 +9488,19 @@ let build_program ~keep_going (decls : Ast.decl list) : Tast.program * env =
|
||||
be written anywhere in it, which is also what makes a reload rebuild
|
||||
every dispatch from the session's declarations — see lib/classes.ml. *)
|
||||
let decls = Classes.expand decls in
|
||||
(* And with the declaration list in its final shape — the classes expanded,
|
||||
the shims flattened, the imports already qualified by [Load] — the one
|
||||
warning this compiler prints unasked. Here rather than in [bin/main.ml]
|
||||
beside [print_memory_warnings] because every route into the compiler
|
||||
passes through this function: build, check, run, and the dev daemon's
|
||||
reload, which is where a defn is most likely to be written. Printed in
|
||||
the shape [Loc] gives an error, so a checker in an editor parses it the
|
||||
same way. *)
|
||||
List.iter
|
||||
(fun (d : Loc.diag) ->
|
||||
prerr_endline
|
||||
(Loc.entry ~mark:'~' ~label:"warning: " d.Loc.dloc d.Loc.dmsg))
|
||||
(shadowed_builtins decls);
|
||||
(* Pass one, and it stops at the first thing it refuses. That is not
|
||||
laziness: every name, type and signature in the file comes from here, so a
|
||||
declaration this pass could not make sense of leaves a hole that pass two
|
||||
|
||||
16
test/dune
16
test/dune
@ -56,6 +56,10 @@
|
||||
(glob_files programs/pkgs/mac/*)
|
||||
(glob_files programs/pkgs/macring/*)
|
||||
(glob_files programs/pkgs/macspin/*)
|
||||
; The package whose body calls the builtin get while the program importing
|
||||
; it defines a get of its own — shadow-builtin.flan, which is the pin that
|
||||
; a shadow stops at the file that declared it.
|
||||
(glob_files programs/pkgs/shadowed/*)
|
||||
; The synthetic C header the importer's table reads. Committed rather than
|
||||
; reached for on the machine: the raylib case needs raylib installed, at the
|
||||
; right version, with a variable set, so it skips everywhere and covers
|
||||
@ -215,7 +219,9 @@
|
||||
; And the macro-declaring packages, for the same reason.
|
||||
(glob_files programs/pkgs/mac/*)
|
||||
(glob_files programs/pkgs/macring/*)
|
||||
(glob_files programs/pkgs/macspin/*))
|
||||
(glob_files programs/pkgs/macspin/*)
|
||||
; And the package shadow-builtin.flan imports.
|
||||
(glob_files programs/pkgs/shadowed/*))
|
||||
(action (run ./test_valgrind.exe)))
|
||||
|
||||
; The corpus a fourth time, through the hand-written x86-64 backend, compared
|
||||
@ -272,7 +278,9 @@
|
||||
(glob_files programs/pkgs/tree/*)
|
||||
(glob_files programs/pkgs/mac/*)
|
||||
(glob_files programs/pkgs/macring/*)
|
||||
(glob_files programs/pkgs/macspin/*))
|
||||
(glob_files programs/pkgs/macspin/*)
|
||||
; And the package shadow-builtin.flan imports.
|
||||
(glob_files programs/pkgs/shadowed/*))
|
||||
(action
|
||||
(setenv SURVEY_STRICT 1
|
||||
(setenv SURVEY_QUIET 1
|
||||
@ -435,7 +443,9 @@
|
||||
(glob_files programs/pkgs/tree/*)
|
||||
(glob_files programs/pkgs/mac/*)
|
||||
(glob_files programs/pkgs/macring/*)
|
||||
(glob_files programs/pkgs/macspin/*))
|
||||
(glob_files programs/pkgs/macspin/*)
|
||||
; And the package shadow-builtin.flan imports.
|
||||
(glob_files programs/pkgs/shadowed/*))
|
||||
(action
|
||||
(setenv SURVEY_STRICT 1
|
||||
(setenv SURVEY_QUIET 1
|
||||
|
||||
10
test/programs/pkgs/shadowed/shadowed.flan
Normal file
10
test/programs/pkgs/shadowed/shadowed.flan
Normal file
@ -0,0 +1,10 @@
|
||||
;;;; A package that calls the builtin get, imported by a program that defines
|
||||
;;;; a get of its own.
|
||||
;;;;
|
||||
;;;; The importer's defn takes the name over in the importer's own file and
|
||||
;;;; nowhere else: this file's names were qualified at the import (this
|
||||
;;;; function is shadowed/field to everything downstream), so the get written
|
||||
;;;; here is the builtin's, was compiled as the builtin's, and answers what a
|
||||
;;;; dyn map holds under a keyword.
|
||||
|
||||
(defn field [m] dyn (get m :b))
|
||||
24
test/programs/shadow-builtin.flan
Normal file
24
test/programs/shadow-builtin.flan
Normal file
@ -0,0 +1,24 @@
|
||||
;;;; A defn named after a builtin, and what the name means afterwards.
|
||||
;;;;
|
||||
;;;; "Allow shadowing but warn": this file's (defn get ...) is legal, it wins
|
||||
;;;; at every call site in this file, and the compiler says so once at the
|
||||
;;;; definition — the warning is on stderr and the exit status does not move.
|
||||
;;;;
|
||||
;;;; Two calls, and between them the whole rule:
|
||||
;;;;
|
||||
;;;; - (get p) is one argument, which the builtin get does not take. It
|
||||
;;;; compiles, and it prints the field, because the name resolves to the
|
||||
;;;; definition below and the builtin is not consulted about its arity.
|
||||
;;;; - (shadowed/field m) reaches into the imported package, whose body calls
|
||||
;;;; the builtin get on a dyn map. The shadow does not follow it there: a
|
||||
;;;; package's own calls mean what they meant when the package was written.
|
||||
|
||||
(import shadowed "pkgs/shadowed")
|
||||
|
||||
(defstruct P [x i32])
|
||||
|
||||
(defn get [p P] i32 (.x p))
|
||||
|
||||
(defn main [] ()
|
||||
(println (get (P {.x 7})))
|
||||
(println (shadowed/field {:a 1 :b 4})))
|
||||
@ -2475,6 +2475,20 @@ let () =
|
||||
shape/Box and not area/shape/Box. *)
|
||||
outputs "a diamond, with a type crossing it" "programs/pkg-diamond.flan"
|
||||
"3\n6\n20\n";
|
||||
(* A defn named after a builtin, and the boundary the shadow stops at.
|
||||
The numbers are the whole claim and neither of them could be printed
|
||||
by the other reading: 7 is the program's own one-argument (get p),
|
||||
which the builtin get has no arity for at all, and 4 is the builtin
|
||||
get called inside the imported package on a dyn map — the same name,
|
||||
in one program, meaning two things because the package's names were
|
||||
qualified at the import.
|
||||
|
||||
The warning the definition earns is on stderr and is pinned in
|
||||
test_flan.ml, where the line and column can be asked about directly.
|
||||
What is asserted here is that it changes nothing else: the program
|
||||
runs and its status is zero. *)
|
||||
outputs "a defn shadows a builtin, and the package it imports does not"
|
||||
"programs/shadow-builtin.flan" "7\n4\n";
|
||||
(* A data type crossing the same boundary, which was a refusal by name
|
||||
until vendor/edn needed one. The rename has two halves and the second
|
||||
is the one that is easy to do by accident only: the type's name is a
|
||||
|
||||
@ -4311,25 +4311,38 @@ let () =
|
||||
"(defn f [] i32 (let [x 1] (.r x)))"
|
||||
~needle:"i32 is not a struct, so it has no fields";
|
||||
|
||||
(* A defn whose name is a builtin's is silently unreachable — the dispatch
|
||||
reaches every builtin arm before it looks in the function table — and the
|
||||
arity refusal was measured against the builtin while pointing at a call
|
||||
the reader had written for their own. *)
|
||||
(match diag_of "(defstruct P [x i32])\n(defn get [p P] i32 (.x p))\n (defn f [] i32 (let [p (P {.x 1})] (get p)))" with
|
||||
| Some d ->
|
||||
check "a shadowed builtin's arity has a kind"
|
||||
(d.Loc.kind = "check/builtin-arity");
|
||||
check "and says whose count it is"
|
||||
(contains d.Loc.dmsg
|
||||
"this is the builtin get, which a defn of the same name does not \
|
||||
replace");
|
||||
(match d.Loc.notes with
|
||||
| [ n ] ->
|
||||
check "and notes the definition that is not being reached"
|
||||
(n.Loc.nloc.Loc.line = 2
|
||||
&& contains n.Loc.nmsg "this call is not reaching it")
|
||||
| _ -> check "a shadowed builtin has one note" false)
|
||||
| None -> check "a shadowed builtin's call is refused" false);
|
||||
(* "Allow shadowing but warn": a defn whose name is a builtin's is legal,
|
||||
it wins at the call sites of the file that wrote it, and the compiler
|
||||
says so once at the definition.
|
||||
|
||||
This used to be the other way round — the dispatch reached every builtin
|
||||
arm before it looked in the function table, so the defn was silently
|
||||
unreachable and the arity refusal carried a note saying so. That note
|
||||
described a resolution order this compiler no longer has, and the source
|
||||
below, which used to be refused, is the one that proves it: (get p) is
|
||||
one argument, and the builtin get takes two. *)
|
||||
let shadow_src =
|
||||
"(defstruct P [x i32])\n(defn get [p P] i32 (.x p))\n\
|
||||
(defn f [] i32 (let [p (P {.x 1})] (get p)))"
|
||||
in
|
||||
(match Check.shadowed_builtins (program shadow_src) with
|
||||
| [ d ] ->
|
||||
check "a defn named after a builtin is warned about, at the definition"
|
||||
(d.Loc.kind = "check/shadows-builtin"
|
||||
&& d.Loc.dloc.Loc.line = 2 && d.Loc.dloc.Loc.col = 7);
|
||||
check "and the warning says what the name now means"
|
||||
(d.Loc.dmsg
|
||||
= "get shadows the builtin get — every call in this program now \
|
||||
reaches your definition");
|
||||
check "and it carries no notes, being one sentence about one decision"
|
||||
(d.Loc.notes = [])
|
||||
| _ -> check "a shadowing defn is warned about exactly once" false);
|
||||
check "and the call reaches the defn, at the defn's arity"
|
||||
(match checked shadow_src with
|
||||
| _ -> true
|
||||
| exception Loc.Error _ -> false);
|
||||
check "a program that shadows nothing is warned at not at all"
|
||||
(Check.shadowed_builtins (program "(defn f [] i32 1)") = []);
|
||||
|
||||
(* and's last operand is the then arm and the sentinel carrying the previous
|
||||
operand's location is the else arm, so with no expectation in hand the
|
||||
@ -4559,10 +4572,17 @@ let () =
|
||||
|
||||
There is no way to reflect over an OCaml match, so this reads the source
|
||||
instead. The two regions are [named_call]'s arms and [var]'s, each from
|
||||
its own [and] down to the first catch-all at the same indentation, and
|
||||
the names are the string literals in the arm heads. It is a regex over
|
||||
one file and costs nothing, which is why it is in the default run rather
|
||||
than behind an alias. *)
|
||||
its own [and] down to the catch-all at the same indentation, and the
|
||||
names are the string literals in the arm heads. It is a regex over one
|
||||
file and costs nothing, which is why it is in the default run rather
|
||||
than behind an alias.
|
||||
|
||||
The catch-all is [ | _ ->] and not [ | _], because a guarded arm is
|
||||
not one: [named_call] opens with [| _ when shadows_builtin ...], which
|
||||
is a name the program defined taking its own call over, and stopping
|
||||
there would read the region as empty and report every builtin as
|
||||
undescribed. Guarded arms in between are skipped by the same rule that
|
||||
skips a comment — they carry no string literal in the head. *)
|
||||
let arm_names () =
|
||||
let src =
|
||||
In_channel.with_open_bin "../lib/check.ml" In_channel.input_all
|
||||
@ -4591,7 +4611,7 @@ let () =
|
||||
let rec take = function
|
||||
| [] -> []
|
||||
| l :: rest ->
|
||||
if starts_with " | _" l then []
|
||||
if starts_with " | _ ->" l then []
|
||||
else if starts_with " | \"" l then quoted l @ take rest
|
||||
else take rest
|
||||
in
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user