fix(lower): pin defining-module context for pack/comptime metaprograms; drop #insert exemption [stdlib B attempt-3]
ROOT FIX for issue 0106's library-metaprogram half — no exemption.
attempt-2 masked the 0106 fallout with an `in_insert_expansion` flag that
made the visibility adapters fall open during ANY `#insert` expansion,
including a USER's `#insert <expr>` — so a bare reach into a namespaced-only
import from user `#insert` code wrongly compiled (Adi's blocker). The flag
was the wrong shape. This removes it and fixes the real cause.
Root cause: a metaprogram's body (`std.print` / `std.format` / `log.*`,
whose `#insert build_format(fmt)` + `#insert "out(result);"` reference
std-internal bare names) was lowered under the CALL SITE's
`current_source_file`, so those names were policed against the consumer's
imports. Normal functions get this right via `lowerFunctionBodyInto`, which
pins `func.source_file`; the two monomorphizers don't:
- `monomorphizePackFn` — bare `print(...)` / `format(...)` (pack path).
- `lowerComptimeCall` — namespaced `std.print` / `log.warn` (reached via
the field-access `hasComptimeParams` branch).
Fix: both paths now save/set/restore `current_source_file` to the body's
DEFINING module around the BODY lowering only (call-site args stay in the
caller's context). The defining path is stamped onto each function body node
by `resolveImports` (`stampFnBodySource`), mirroring `Function.source_file`.
So library internals resolve in std.sx/log.sx naturally, while a USER's
`#insert <expr>` is still checked in the user's context.
- Exemption GONE: `in_insert_expansion` flag + both adapter fall-open checks
deleted; `isNameVisible`/`isCImportVisible` are byte-identical adapters.
- New pinned regression: examples/0737-modules-insert-bare-not-visible.sx
(+ a.sx) — a USER `#insert secret()` into a namespaced-only import errors
('secret' is not visible). fail-before exit 0 on the attempt-2 binary /
pass-after exit 1.
- face #1 (0736) still errors; face #2 (0015/0700/0718/1030) pass again WITH
NO exemption — the metaprogram body resolves in its own module.
- run_examples 472 -> 473; zig build test 412/412; m3te ios-sim build exit 0.
- issues/0106 RESOLVED banner updated (root cause + no-exemption fix).
This commit is contained in:
15
examples/0737-modules-insert-bare-not-visible.sx
Normal file
15
examples/0737-modules-insert-bare-not-visible.sx
Normal file
@@ -0,0 +1,15 @@
|
||||
// A bare name inside a USER `#insert <expr>` is visibility-checked in the
|
||||
// USER's module, not skipped (regression, issue 0106). `a.sx` is imported only
|
||||
// as `m :: #import` — its top-level `secret` is reachable ONLY as `m.secret`.
|
||||
// A BARE `secret()` driving a `#insert` must error just like any other bare
|
||||
// reference into a namespaced-only import: `#insert` expansion does NOT exempt
|
||||
// user-typed code from visibility. (Library metaprograms like `std.print` keep
|
||||
// working because their bodies lower in their OWN module's context — see
|
||||
// `monomorphizePackFn` / `lowerComptimeCall` pinning `current_source_file` to
|
||||
// the body's defining module — not because `#insert` is exempt.)
|
||||
m :: #import "0737-modules-insert-bare-not-visible/a.sx";
|
||||
|
||||
main :: () -> s32 {
|
||||
#insert secret();
|
||||
0
|
||||
}
|
||||
1
examples/0737-modules-insert-bare-not-visible/a.sx
Normal file
1
examples/0737-modules-insert-bare-not-visible/a.sx
Normal file
@@ -0,0 +1 @@
|
||||
secret :: () -> string { "leaked := 1;" }
|
||||
@@ -0,0 +1 @@
|
||||
1
|
||||
@@ -0,0 +1,5 @@
|
||||
error: 'secret' is not visible; #import the module that declares it
|
||||
--> examples/0737-modules-insert-bare-not-visible.sx:13:13
|
||||
|
|
||||
13 | #insert secret();
|
||||
| ^^^^^^
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# 0106 — namespaced-import internal names are silently bare-visible (over-permissive `isNameVisible`)
|
||||
|
||||
> **RESOLVED** (flow stdlib/B attempt-2). Two coupled changes:
|
||||
> **RESOLVED** (flow stdlib/B attempt-3 — root fix, no exemption). Two coupled
|
||||
> changes:
|
||||
>
|
||||
> 1. **Tightened bare visibility to the flat edge set.** `isNameVisible` /
|
||||
> `isCImportVisible` now route through the unified `isVisible` predicate over
|
||||
@@ -8,28 +9,41 @@
|
||||
> `import_graph`). A namespaced-only import's internal name is no longer
|
||||
> bare-visible — face #1 now errors `'<name>' is not visible; #import the
|
||||
> module that declares it`.
|
||||
> 2. **`#insert`-expansion visibility exemption.** The flat tightening alone broke
|
||||
> `std.print` / `log.*`: a library metaprogram's `#insert build_format(fmt)`
|
||||
> (comptime call) and `#insert "out(result);"` (inserted statement) expand in
|
||||
> the CALL SITE's `current_source_file`, so their bare names (`build_format`,
|
||||
> `out`, `emit`) were policed against the consumer's imports. Fix: a precise,
|
||||
> named exemption — `Lowering.in_insert_expansion` is set across
|
||||
> `lowerInsertExprValue` (the comptime eval + the parsed-back statements), and
|
||||
> `isNameVisible` / `isCImportVisible` fall open while it is set. This mirrors
|
||||
> the existing UFCS-alias / mangled-local "compiler indirection" exemptions; it
|
||||
> is NOT a blanket skip (it scopes to `#insert`-expanded code; ordinary bare
|
||||
> references are still policed). Library-internal call bodies (e.g.
|
||||
> `build_format`'s `concat` / `substr`) already resolve correctly — they lower
|
||||
> via `lowerFunctionBodyInto`, which pins `current_source_file` to the defining
|
||||
> module.
|
||||
> 2. **Pin the defining-module context during pack/comptime monomorphization.**
|
||||
> The flat tightening alone broke `std.print` / `log.*`: a library metaprogram's
|
||||
> body (`#insert build_format(fmt)` comptime call + the `#insert "out(result);"`
|
||||
> inserted statement) was lowered under the CALL SITE's `current_source_file`,
|
||||
> so its bare names (`build_format`, `out`, `emit`) were policed against the
|
||||
> consumer's imports. **Root cause:** `monomorphizePackFn` (bare `print` /
|
||||
> `format`) and `lowerComptimeCall` (namespaced `std.print` / `log.*`, reached
|
||||
> via the field-access `hasComptimeParams` branch) lower the metaprogram body
|
||||
> without pinning the source context — unlike a normal function, which lowers
|
||||
> via `lowerFunctionBodyInto` pinning `func.source_file`. **Fix:** both paths
|
||||
> now save/set/restore `current_source_file` to the body's DEFINING module
|
||||
> before lowering the body (the call-site ARGS are lowered first, in the
|
||||
> caller's context, which is correct). The defining path is stamped onto each
|
||||
> function body node by `resolveImports` (`stampFnBodySource`, mirroring how a
|
||||
> declared function carries `Function.source_file`). So the metaprogram's bare
|
||||
> `build_format` / `out` / `emit` resolve in `std.sx` / `log.sx` naturally —
|
||||
> and a USER's `#insert <expr>` is still checked in the USER's context, so a
|
||||
> bare reach into a namespaced-only import there errors. **No `#insert`
|
||||
> exemption** (attempt-2's `in_insert_expansion` flag is deleted): the fix is
|
||||
> the absence of an exemption, not a narrower one.
|
||||
>
|
||||
> Root cause: `isNameVisible` walked `import_graph` (flat AND namespaced edges)
|
||||
> where a bare name should join only over `flat_import_graph`.
|
||||
> Regression: `examples/0736-modules-namespaced-only-bare-not-visible.sx` (+
|
||||
> `0736-…/a.sx`) — face #1 pinned (exit 1 + the stderr). Face #2 restored:
|
||||
> `examples/0015 / 0700 / 0718 / 1030` pass again (`run_examples` 471 → 472).
|
||||
> Fix in `src/ir/lower.zig` (`in_insert_expansion` + the two adapters) +
|
||||
> `src/ir/resolver.zig` (`VisibilityMode` modes, landed in attempt-1).
|
||||
> where a bare name should join only over `flat_import_graph`; and the pack /
|
||||
> comptime monomorphizers lowered the metaprogram body under the wrong source
|
||||
> context.
|
||||
> Regressions: `examples/0736-modules-namespaced-only-bare-not-visible.sx` (+
|
||||
> `0736-…/a.sx`) — face #1 pinned (exit 1 + the stderr);
|
||||
> `examples/0737-modules-insert-bare-not-visible.sx` (+ `0737-…/a.sx`) — a USER
|
||||
> `#insert secret()` into a namespaced-only import errors (fail-before exit 0 on
|
||||
> the attempt-2 exemption / pass-after exit 1). Face #2 restored WITHOUT an
|
||||
> exemption: `examples/0015 / 0700 / 0718 / 1030` pass again (`run_examples`
|
||||
> 471 → 473). Fix in `src/ir/lower.zig` (`monomorphizePackFn` +
|
||||
> `lowerComptimeCall` source-context pin; exemption removed) + `src/imports.zig`
|
||||
> (`stampFnBodySource`) + `src/ir/resolver.zig` (`VisibilityMode` modes, landed in
|
||||
> attempt-1).
|
||||
|
||||
**Symptom.** A bare reference to a top-level name authored in a module that the
|
||||
consumer imports **only namespaced** (`ns :: #import "m.sx"`) is silently
|
||||
|
||||
@@ -591,6 +591,25 @@ fn reportDuplicateName(diagnostics: ?*errors.DiagnosticList, added: bool, name:
|
||||
diags.addFmt(.err, span, "duplicate top-level declaration '{s}'", .{name});
|
||||
}
|
||||
|
||||
/// Stamp the DEFINING module path onto a function body node, so a later
|
||||
/// pack/comptime monomorphization can pin `current_source_file` to the body's
|
||||
/// own module and resolve its bare names in that module's visibility context
|
||||
/// (issue 0106) — mirroring how a normally-declared function carries
|
||||
/// `Function.source_file`. Only top-level decl Nodes are otherwise stamped, so
|
||||
/// the body Node would carry no source; a null body source after this means a
|
||||
/// synthesized/sourceless decl (the monomorphizer then keeps its caller's
|
||||
/// context, the legitimate fall-open).
|
||||
fn stampFnBodySource(decl: *Node, file_path: []const u8) void {
|
||||
switch (decl.data) {
|
||||
.fn_decl => |fd| fd.body.source_file = file_path,
|
||||
.const_decl => |cd| switch (cd.value.data) {
|
||||
.fn_decl => |fd| fd.body.source_file = file_path,
|
||||
else => {},
|
||||
},
|
||||
else => {},
|
||||
}
|
||||
}
|
||||
|
||||
/// `reportDuplicateName` keyed off a node whose `declName()` carries the name
|
||||
/// (the regular authored-decl sites; an `import_decl` has no `declName`, so a
|
||||
/// namespace alias must use `reportDuplicateName` with the alias directly).
|
||||
@@ -746,6 +765,7 @@ pub fn resolveImports(
|
||||
}
|
||||
if (decl.data != .import_decl) {
|
||||
decl.source_file = file_path;
|
||||
stampFnBodySource(decl, file_path);
|
||||
reportDuplicateDecl(diagnostics, try mod.addOwnDecl(allocator, &decl_list, &own_decl_list, &seen_in_list, decl), decl);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -169,17 +169,6 @@ pub const Lowering = struct {
|
||||
/// `self.program_index.<field>`; populated by scan/registration code.
|
||||
program_index: ProgramIndex,
|
||||
current_source_file: ?[]const u8 = null, // source file of function currently being lowered
|
||||
/// True while lowering the product of a `#insert` expansion — the comptime
|
||||
/// call whose string drives the insert, plus the statements parsed back from
|
||||
/// that string. Bare names in this synthesized code are authored by the
|
||||
/// library metaprogram (e.g. `out`/`emit`/`build_format` inside `std.print`),
|
||||
/// not user-typed at the call site, so the bare-name `#import` visibility
|
||||
/// adapters (`isNameVisible` / `isCImportVisible`) exempt them — the same
|
||||
/// "compiler indirection" exemption already given to UFCS-alias rewrites and
|
||||
/// mangled local names (issue 0106). It is NOT a blanket skip: it scopes the
|
||||
/// exemption to `#insert`-expanded code only; ordinary bare references still
|
||||
/// get policed. Saved/restored to nest correctly.
|
||||
in_insert_expansion: bool = false,
|
||||
// Implicit Context parameter machinery. When the program imports
|
||||
// `std.sx` (and therefore declares `Context :: struct {...}`), every
|
||||
// default-conv sx function gains a synthetic `__sx_ctx: *void` param
|
||||
@@ -1857,19 +1846,14 @@ pub const Lowering = struct {
|
||||
|
||||
/// Check if a C-imported function is visible from the current source file.
|
||||
/// Returns true for non-C functions (always visible) or if no scoping info
|
||||
/// available. Adapter over `isVisible(.c_import_bare)`, plus the
|
||||
/// `#insert`-expansion exemption (issue 0106).
|
||||
/// available. Byte-identical adapter over `isVisible`.
|
||||
fn isCImportVisible(self: *Lowering, fn_name: []const u8) bool {
|
||||
if (self.in_insert_expansion) return true;
|
||||
return self.isVisible(fn_name, .c_import_bare);
|
||||
}
|
||||
|
||||
/// Non-transitive `#import` visibility check for top-level decls. Adapter
|
||||
/// over `isVisible(.user_bare_flat)`, plus the `#insert`-expansion exemption:
|
||||
/// names emitted by a library metaprogram's insert are compiler
|
||||
/// indirections, not user-typed call-site references (issue 0106).
|
||||
/// Non-transitive `#import` visibility check for top-level decls.
|
||||
/// Byte-identical adapter over `isVisible`.
|
||||
fn isNameVisible(self: *Lowering, name: []const u8) bool {
|
||||
if (self.in_insert_expansion) return true;
|
||||
return self.isVisible(name, .user_bare_flat);
|
||||
}
|
||||
|
||||
@@ -9496,14 +9480,6 @@ pub const Lowering = struct {
|
||||
|
||||
/// Like lowerInsertExpr but returns the value of the last parsed expression.
|
||||
fn lowerInsertExprValue(self: *Lowering, expr: *const Node) Ref {
|
||||
// The comptime call that produces the insert string and the statements
|
||||
// parsed back from it are library-metaprogram code, not user-typed bare
|
||||
// names at this call site — exempt them from the `#import` visibility
|
||||
// adapters (issue 0106). Saved/restored so nested `#insert`s compose.
|
||||
const saved_insert = self.in_insert_expansion;
|
||||
self.in_insert_expansion = true;
|
||||
defer self.in_insert_expansion = saved_insert;
|
||||
|
||||
// Step 1: Substitute comptime param nodes (e.g., replace $fmt with its literal)
|
||||
const substituted = if (self.comptime_param_nodes) |cpn|
|
||||
self.substituteComptimeNodes(expr, cpn) catch expr
|
||||
@@ -9701,6 +9677,19 @@ pub const Lowering = struct {
|
||||
self.pack_arg_nodes = saved_pan;
|
||||
}
|
||||
|
||||
// Pin the lowering to the metaprogram's OWN module for the body (and
|
||||
// its return type + anything it `#insert`s, e.g. `build_format` / `out`
|
||||
// / `emit` inside `std.print` / `log.*`), so those bare names resolve
|
||||
// in the defining module's visibility context rather than the call
|
||||
// site's (issue 0106). The call-site ARGS above are deliberately lowered
|
||||
// BEFORE this, in the caller's context. Mirrors `lowerFunctionBodyInto`,
|
||||
// which switches to `func.source_file`. The defining path is stamped on
|
||||
// the body node by `resolveImports`; a sourceless body keeps the
|
||||
// caller's context.
|
||||
const saved_source = self.current_source_file;
|
||||
defer self.setCurrentSourceFile(saved_source);
|
||||
if (fd.body.source_file) |src| self.setCurrentSourceFile(src);
|
||||
|
||||
// Lower the body — capture return value for functions with return type
|
||||
const ret_ty = self.resolveReturnType(fd);
|
||||
if (ret_ty != .void) {
|
||||
@@ -10924,6 +10913,18 @@ pub const Lowering = struct {
|
||||
// concrete per-position types.
|
||||
self.materialisePackSlice(&scope, pack_name, pack_param_slots.items, arg_types);
|
||||
|
||||
// Pin to the metaprogram's OWN module for the BODY lowering only, so its
|
||||
// bare names (and anything it `#insert`s — e.g. `build_format` / `out` /
|
||||
// `emit` inside `std.print`) resolve in the defining module's visibility
|
||||
// context, not the call site's (issue 0106). The comptime-param call-site
|
||||
// args above were deliberately lowered FIRST, in the caller's context.
|
||||
// Mirrors `lowerFunctionBodyInto`, which switches to `func.source_file`;
|
||||
// the defining path is stamped on the body node by `resolveImports`. A
|
||||
// synthesized/sourceless body keeps the caller's context.
|
||||
const saved_source = self.current_source_file;
|
||||
defer self.setCurrentSourceFile(saved_source);
|
||||
if (fd.body.source_file) |src| self.setCurrentSourceFile(src);
|
||||
|
||||
if (ret_ty != .void) {
|
||||
const body_val = self.lowerBlockValue(fd.body);
|
||||
if (!self.currentBlockHasTerminator()) {
|
||||
|
||||
Reference in New Issue
Block a user