diff --git a/crates/oak_db/src/file.rs b/crates/oak_db/src/file.rs index a6f31e254..04205f27f 100644 --- a/crates/oak_db/src/file.rs +++ b/crates/oak_db/src/file.rs @@ -2,7 +2,10 @@ use std::fs; use std::sync::Arc; use aether_path::FilePath; +use biome_line_index::LineIndex; +use biome_rowan::TextRange; use oak_semantic::semantic_index::ScopeId; +use oak_semantic::semantic_index::SemanticDiagnostic; use oak_semantic::semantic_index::SemanticIndex; use oak_semantic::semantic_index::SymbolTable; use oak_semantic::use_def_map::UseDefMap; @@ -317,7 +320,45 @@ fn root_by_path(db: &dyn Db, path: &FilePath) -> Option { fn build_semantic_index(file: File, db: &dyn Db) -> SemanticIndex { let parsed = file.parse(db); let resolver = SalsaImportsResolver::new(db, file); - oak_semantic::build_index(&parsed.tree(), resolver) + let index = oak_semantic::build_index(&parsed.tree(), resolver); + + // TODO(diagnostics): Diagnostics are not surfaced yet, so log them for now. + // The builder is file-agnostic, so it carries them on the index and leaves + // the file reference to us. + let diagnostics = index.diagnostics(); + if !diagnostics.is_empty() { + let path = file.path(db); + let line_index = file.line_index(db); + + for diagnostic in diagnostics { + match diagnostic { + SemanticDiagnostic::LazyShadowAmbiguity { + name, + call_range, + overwrite_range, + } => { + let call = format_line_col(line_index, *call_range); + let overwrite = format_line_col(line_index, *overwrite_range); + log::warn!( + "Lazy-shadow ambiguity in {path}:{call}: callee `{name}` is recognized \ + as effectful, but a lazy-crossed ancestor binds it at {overwrite} with \ + undetermined timing" + ) + }, + } + } + } + + index +} + +/// Render a byte range as `line:col` (1-based), anchored at its start, for a log +/// message. Falls back to the raw byte range if the offset can't be mapped. +fn format_line_col(line_index: &LineIndex, range: TextRange) -> String { + match line_index.line_col(range.start()) { + Some(pos) => format!("{}:{}", pos.line + 1, pos.col + 1), + None => format!("{range:?}"), + } } fn semantic_index_cycle_result(db: &dyn Db, _id: salsa::Id, file: File) -> SemanticIndex { diff --git a/crates/oak_semantic/src/builder.rs b/crates/oak_semantic/src/builder.rs index 14b559f54..5aa29b155 100644 --- a/crates/oak_semantic/src/builder.rs +++ b/crates/oak_semantic/src/builder.rs @@ -39,18 +39,18 @@ use oak_core::syntax_ext::RStringValueExt; use oak_index_vec::Idx; use oak_index_vec::IndexVec; use rustc_hash::FxHashMap; -use scan::BoundNames; +use scan::BindingSites; +use scan::BodyScan; use scan::CallResolution; -use scan::EagerNestedDescent; +use scan::DeferredBody; use scan::FlowState; +use scan::OpenScope; use crate::resolver::ImportsResolver; use crate::semantic_index::Definition; use crate::semantic_index::DefinitionId; use crate::semantic_index::EnclosingSnapshotId; use crate::semantic_index::EnclosingSnapshotKey; -use crate::semantic_index::EvalEnv; -use crate::semantic_index::EvalTiming; use crate::semantic_index::NamespaceAccess; use crate::semantic_index::Scope; use crate::semantic_index::ScopeId; @@ -85,6 +85,7 @@ pub fn build_index(root: &RRoot, resolver: impl ImportsResolver) -> SemanticInde let mut builder = SemanticIndexBuilder::new(range, resolver); builder.begin_scan(); builder.scan_expression_list(&root.expressions()); + builder.scan_deferred_bodies(0); builder.walk_expression_list(&root.expressions()); builder.finish() } @@ -103,34 +104,46 @@ struct SemanticIndexBuilder { walk: WalkState, } -/// State owned by the scan pass: its working state plus the products the walk -/// reads back (`bound_names`, `call_resolutions`, `eager_descent.pending`). -/// The walk also writes `bound_names`, but only to install scan-produced data: -/// the lockstep push in `push_scope()` and the pending install in -/// `walk_nse_argument()`. +/// State owned by the scan pass. +/// +/// Binding state comes in two views, because eager and lazy code ask +/// different questions. +/// +/// - An eager callee is shadowed only by bindings that already ran. +/// `bound_so_far` reflects this view. It rewinds at branch joins and is +/// reseeded for each scan unit. +/// - A lazy body runs after its scope has finished and resolves symbols +/// in the whole scope. `bound_anywhere` reflects this view. +/// +/// Both views are written together by `record_binding()`. They diverge on +/// two rules. Names inherited from enclosing scopes seed `bound_so_far` only, +/// via `begin_scan()`. Names bound by a deferred body reach the owner's +/// `bound_anywhere` only, because the deferred scan restores `bound_so_far` +/// afterwards, so the name is visible to lazy readers without shadowing an +/// eager callee after the call. struct ScanState { - bound_names: IndexVec, - // Per-call facts resolved by the scanner in flow order, keyed by the call's - // range. See `CallResolution`. - call_resolutions: FxHashMap, - // The scan's flow-precise binding state for the scope being scanned, reset - // at each scope's `begin_scan()`. See [`FlowState`]. - flow_state: FlowState, - // Names inherited from enclosing scopes at this scope's entry point, keyed - // by the scope's range. Captured from `flow_state`, and read by - // `begin_scan()` to seed the scope's own scan. - enclosing_flow: FxHashMap, + bound_anywhere: IndexVec, + bound_so_far: FlowState, + // Scopes the scan has entered that are not yet allocated in the arena, + // innermost last. + open_scopes: Vec, + // What the scan prepared for each child body, keyed by the body's range. + // See [`BodyScan`]. + body_scans: FxHashMap, // Packages attached in eager flow order (file level and eager NSE descents), // appended only when `!is_lazy()`. Append-only, never restored across a // descent or branch: attaches hit the global search path, they aren't scoped - // like `flow_state`. An eager callee reads the flow-precise prefix during + // like `bound_so_far`. An eager callee reads the flow-precise prefix during // the file scan. A lazy callee reads the complete set during the walk (which // runs after the file scan finishes), so this doubles as the end-of-file // attach view. attached_flow: Vec, - // Bound names of Eager + Nested bodies like `local()` are discovered inline - // by the scanner. See `EagerNestedDescent`. - eager_descent: EagerNestedDescent, + // Per-call facts resolved by the scanner in flow order, keyed by the call's + // range. See `CallResolution`. + call_resolutions: FxHashMap, + // `Current + Lazy` bodies (e.g. `rlang::on_load()`) queued at their call + // sites, scanned when their enclosing scan unit finishes. + deferred_bodies: Vec, } /// State written by the walk pass: the per-scope arenas and the flat outputs @@ -156,7 +169,7 @@ impl SemanticIndexBuilder { let mut definitions = IndexVec::new(); let mut uses = IndexVec::new(); let mut use_def_maps = IndexVec::new(); - let mut bound_names = IndexVec::new(); + let mut bound_anywhere = IndexVec::new(); // The descendants range starts empty (`n+1..n+1`). `pop_scope` later // fills in `descendants.end` with the current arena length. Everything @@ -175,7 +188,7 @@ impl SemanticIndexBuilder { definitions.push(IndexVec::new()); uses.push(IndexVec::new()); use_def_maps.push(UseDefMapBuilder::new()); - bound_names.push(BoundNames::new()); + bound_anywhere.push(BindingSites::new()); Self { scopes, @@ -183,12 +196,13 @@ impl SemanticIndexBuilder { diagnostics: Vec::new(), resolver, scan: ScanState { - bound_names, + bound_anywhere, call_resolutions: FxHashMap::default(), - flow_state: FlowState::default(), - enclosing_flow: FxHashMap::default(), + bound_so_far: FlowState::default(), + body_scans: FxHashMap::default(), attached_flow: Vec::new(), - eager_descent: EagerNestedDescent::default(), + open_scopes: Vec::new(), + deferred_bodies: Vec::new(), }, walk: WalkState { symbol_tables, @@ -223,7 +237,7 @@ impl SemanticIndexBuilder { self.walk.definitions.push(IndexVec::new()); self.walk.uses.push(IndexVec::new()); self.walk.use_def_maps.push(UseDefMapBuilder::new()); - self.scan.bound_names.push(BoundNames::new()); + self.scan.bound_anywhere.push(BindingSites::new()); id } @@ -242,12 +256,9 @@ impl SemanticIndexBuilder { /// climb is iterative to handle e.g. `on_load(on_load(...))`. Every other /// scope kind (`File`, `Function`, `Nse(Nested, _)`) owns its definitions /// and stops the climb. - fn definition_owner(&self) -> Option { + fn enclosing_owner(&self) -> Option { let mut scope = self.scopes[self.current_scope].parent?; - while matches!( - self.scopes[scope].kind, - ScopeKind::Nse(EvalEnv::Current, EvalTiming::Lazy) - ) { + while !self.scopes[scope].kind.owns_bindings() { scope = self.scopes[scope].parent?; } Some(scope) @@ -257,18 +268,18 @@ impl SemanticIndexBuilder { /// already-recorded `IS_BOUND` definition or a pre-scanned assignment. The /// pre-scan covers definitions the walk hasn't reached yet in this scope. fn scope_binds_anywhere(&self, scope: ScopeId, name: &str) -> bool { - self.walked_binding(scope, name).is_some() || self.scan.bound_names[scope].binds(name) + self.walked_binding(scope, name).is_some() || self.scan.bound_anywhere[scope].binds(name) } /// The site where `scope` binds `name`, matching what /// [`scope_binds_anywhere`](Self::scope_binds_anywhere) counts as a binding /// (so it returns `Some` on exactly the same names). Prefers the - /// scan-collected site in `bound_names`, falling back to the range of an + /// scan-collected site in `bound_anywhere`, falling back to the range of an /// already-walked `IS_BOUND` definition (e.g. a parameter, which the scan - /// seeds straight into `flow_state` without a `bound_names` entry). Used to + /// seeds straight into `bound_so_far` without a `bound_anywhere` entry). Used to /// point the lazy-shadow diagnostic at the overwrite. fn scope_binding_range(&self, scope: ScopeId, name: &str) -> Option { - if let Some(range) = self.scan.bound_names[scope].binding_range(name) { + if let Some(range) = self.scan.bound_anywhere[scope].binding_range(name) { return Some(range); } @@ -295,21 +306,6 @@ impl SemanticIndexBuilder { fn finish(mut self) -> SemanticIndex { self.scopes[ScopeId::from(0)].descendants.end = self.scopes.next_id(); - // TODO(diagnostics): Diagnostics are not surfaced yet, so log them for now - for diagnostic in &self.diagnostics { - match diagnostic { - SemanticDiagnostic::LazyShadowAmbiguity { - name, - call_range, - overwrite_range, - } => log::warn!( - "Lazy-shadow ambiguity: callee `{name}` at {call_range:?} is recognized \ - as effectful, but a lazy-crossed ancestor binds it at {overwrite_range:?} \ - with undetermined timing" - ), - } - } - let symbol_tables = self .walk .symbol_tables diff --git a/crates/oak_semantic/src/builder/effects.rs b/crates/oak_semantic/src/builder/effects.rs index e2835a779..c8c94b74c 100644 --- a/crates/oak_semantic/src/builder/effects.rs +++ b/crates/oak_semantic/src/builder/effects.rs @@ -16,6 +16,7 @@ use crate::effects::EffectSite; use crate::effects::Effects; use crate::effects::EffectsHandlers; use crate::resolver::ImportsResolver; +use crate::semantic_index::ScopeId; use crate::semantic_index::SemanticDiagnostic; impl SemanticIndexBuilder { @@ -101,13 +102,13 @@ impl SemanticIndexBuilder { // First check for a local definition (which in the future may // carry declared effects that we resolve here) // - // Looked up from `flow_state` which already carries every + // Looked up from `bound_so_far` which already carries every // eager binding visible here: the scope's own flow-precise // bindings so far, plus the enclosing eager environment seeded // at `begin_scan()`. Forward and deferred (lazy-routed) - // bindings are excluded. A forward one isn't in `flow_state` + // bindings are excluded. A forward one isn't in `bound_so_far` // yet, and a deferred one (`on_load`, `<<-`) never enters it. - if self.scan.flow_state.is_bound(sym) { + if self.scan.bound_so_far.is_bound(sym) { return self.resolve_local_effects(sym); } @@ -117,13 +118,8 @@ impl SemanticIndexBuilder { return None; } - // Now check imports since the symbol is locally unbound. The - // arena's `current_scope` is the scan unit's scope (the descent - // pushes no arena scopes), so its laziness is the "am I in a lazy - // context" test the resolver needs. `attached_flow` is the - // flow-precise attach prefix during the file scan and the - // complete end-of-file set during the walk. - let lazy = self.scopes[self.current_scope].kind.is_lazy(); + // Now check imports since the symbol is locally unbound + let lazy = self.scan_is_lazy(); let effects = self .resolver .resolve_effects(sym, &self.scan.attached_flow, lazy)?; @@ -186,32 +182,88 @@ impl SemanticIndexBuilder { /// /// We've recognized an effect for `name` (NSE scope or attach) because it /// was locally unbound at the current flow cursor and eager-flow resolution - /// found one. If we're in a lazy context, that decision could be wrong: an - /// enclosing scope may bind `name` with a timing we can't pin down, either a - /// later assignment, or one from another deferred body that could run before - /// or after us. We detect this ambiguity here so it can be linted. + /// found an effect. If we're in a lazy context, that decision could be + /// wrong: an enclosing scope may bind `name` with a timing we can't pin + /// down, either a later assignment, or one from another deferred body that + /// could run before or after us. We detect this ambiguity here so it can be + /// linted. /// /// Returns the site of the shadowing binding. fn is_lazily_shadowed(&self, name: &str) -> Option { - let mut scope = self.current_scope; - let mut crossed_lazy = self.scopes[scope].kind.is_lazy(); + let mut open_scopes = self.scan.open_scopes.iter().rev(); + match open_scopes.next() { + // Search the body's ancestors from the inside out for a binding of + // `name` we can't order against the body (see the doc above). Here + // the body is the innermost open scope, e.g. a `local()` / + // `on_load()` body the scan entered before the walk gave it an + // arena scope. Its ancestors are the frames beneath it, then the + // arena scopes from `current_scope` out (included). The `None` arm + // is the mirror case, where `current_scope` is the body itself and + // the walk starts at its parent. + Some(body) => { + let mut crossed_lazy = body.kind.is_lazy(); + for scope in open_scopes { + if crossed_lazy { + if let Some(range) = scope.bindings.binding_range(name) { + return Some(range); + } + } + if scope.kind.is_lazy() { + crossed_lazy = true; + } + } + + self.lazy_shadow_in_arena(name, Some(self.current_scope), crossed_lazy) + }, + + // No frames: the body is `current_scope` itself (a function or + // other lazy context like `reactive()`, scanned at walk time), so + // its ancestors start at its parent. + None => self.lazy_shadow_in_arena( + name, + self.scopes[self.current_scope].parent, + self.scopes[self.current_scope].kind.is_lazy(), + ), + } + } + + /// Walk arena scopes outward from `start`, returning the first that binds + /// `name` after a lazy boundary has been crossed. + fn lazy_shadow_in_arena( + &self, + name: &str, + start: Option, + mut crossed_lazy: bool, + ) -> Option { + let mut scope = start; - while let Some(parent) = self.scopes[scope].parent { + while let Some(s) = scope { if crossed_lazy { - if let Some(range) = self.scope_binding_range(parent, name) { + if let Some(range) = self.scope_binding_range(s, name) { return Some(range); } } - - if self.scopes[parent].kind.is_lazy() { + if self.scopes[s].kind.is_lazy() { crossed_lazy = true; } - scope = parent; + scope = self.scopes[s].parent; } None } + /// Whether the scan is currently inside a lazy context. + /// + /// Laziness is monotone from the outside in. Once an enclosing context runs + /// lazily, everything nested in it does too, even an eager `local()`. + fn scan_is_lazy(&self) -> bool { + self.scopes[self.current_scope].kind.is_lazy() || + self.scan + .open_scopes + .iter() + .any(|frame| frame.kind.is_lazy()) + } + fn record_lazy_shadow_ambiguity( &mut self, name: String, diff --git a/crates/oak_semantic/src/builder/scan.rs b/crates/oak_semantic/src/builder/scan.rs index dd2972918..0992e6df0 100644 --- a/crates/oak_semantic/src/builder/scan.rs +++ b/crates/oak_semantic/src/builder/scan.rs @@ -27,7 +27,7 @@ use super::SemanticIndexBuilder; use crate::effects::AssignBinding; use crate::effects::ResolvedArgumentEffect; use crate::effects::ResolvedArgumentEffects; -use crate::effects::ScopeBindings; +use crate::effects::ScopeContext; use crate::resolver::ImportsResolver; use crate::resolver::SourceResolution; use crate::semantic_index::EvalEnv; @@ -44,9 +44,9 @@ impl SemanticIndexBuilder { /// Seeds it with two things: /// /// - The names inherited from enclosing scopes, captured when this scope was - /// entered (`enclosing_flow`). The parent's own scan was seeded the same - /// way, so this is transitively complete: it holds every eager binding - /// visible from an ancestor at this scope's definition point. + /// entered (its `BodyScan::Deferred` snapshot). The parent's own scan was + /// seeded the same way, so this is transitively complete: it holds every + /// eager binding visible from an ancestor at this scope's definition point. /// - The scope's own already-bound names. For a function scope that's the /// parameters, recorded just before the scan runs. For file and NSE scopes /// nothing local is bound yet. @@ -57,14 +57,24 @@ impl SemanticIndexBuilder { pub(super) fn begin_scan(&mut self) { let range = self.scopes[self.current_scope].range; - match self.scan.enclosing_flow.get(&range).cloned() { - Some(entry) => self.scan.flow_state.restore(entry), - None => self.scan.flow_state.clear(), + match self.scan.body_scans.get(&range) { + // The file scope has no entry: nothing is inherited, so start clean. + None => self.scan.bound_so_far.clear(), + Some(BodyScan::Deferred(snapshot)) => { + let snapshot = snapshot.clone(); + self.scan.bound_so_far.restore(snapshot); + }, + Some(BodyScan::Scanned(_)) => { + // A body scanned inline by an eager descent is installed by the + // walk without a re-scan, so `begin_scan()` should never meet one. + stdext::debug_panic!("`begin_scan()` on an already-scanned body at {range:?}"); + self.scan.bound_so_far.clear(); + }, } for (_id, symbol) in self.walk.symbol_tables[self.current_scope].iter() { if symbol.flags().contains(SymbolFlags::IS_BOUND) { - self.scan.flow_state.bind(symbol.name().to_string()); + self.scan.bound_so_far.bind(symbol.name().to_string()); } } } @@ -80,7 +90,7 @@ impl SemanticIndexBuilder { /// Runs before the walk of a scope. It decides NSE-ness at each call the /// same way the walk's [`is_locally_bound`](Self::is_locally_bound) would, /// records the decision in `call_resolutions` for the walk to reuse, and adds - /// non-skipped definition names to `bound_names`. The bound names must be + /// non-skipped definition names to `bound_anywhere`. The bound names must be /// complete before the walk descends into any child scope, because a lazy /// child body can reference an ancestor def the ancestor's walk hasn't /// reached yet. @@ -92,11 +102,14 @@ impl SemanticIndexBuilder { /// - A `Current + Eager` body pushes no scope, so it stays part of this /// scope's direct level and is scanned through transparently. /// - A `Nested + Eager` body is descended into with a save/restore of - /// `flow_state`, and the names it binds are left pending for the walk to - /// install without re-scanning. - /// - Function and lazy bodies (`Nested + Lazy`, `Current + Lazy`) are their - /// own scan units, scanned separately when the walk enters them, because - /// NSE resolution there needs the child's own flow context. + /// `bound_so_far`, and the names it binds are staged as + /// `BodyScan::Scanned` for the walk to install without re-scanning. + /// - A `Current + Lazy` body (`on_load()`) binds into the owner scope but + /// runs later, so it is queued and scanned at the end of this unit, once + /// the owner's `bound_anywhere` is complete (see `scan_deferred_bodies()`). + /// - Function and `Nested + Lazy` bodies are their own scan units, scanned + /// separately when the walk enters them, because NSE resolution there needs + /// the child's own flow context. /// /// Branch analysis is precise. In `if (c) local <- f else local({ y <- 1 /// })` the else branch sees an NSE call because `local` is unbound on the @@ -107,7 +120,7 @@ impl SemanticIndexBuilder { // A function body is a child scope, scanned when it's entered. // Record the names it inherits now so that when we later resolve // an NSE callee inside the body, we can check whether one of them - // shadows it (see `enclosing_flow`). + // shadows it (see `record_enclosing_flow()`). self.record_enclosing_flow(func.syntax().text_trimmed_range()); }, @@ -188,14 +201,14 @@ impl SemanticIndexBuilder { self.scan_expression(&condition); } - let pre_if = self.scan.flow_state.snapshot(); + let pre_if = self.scan.bound_so_far.snapshot(); if let Ok(consequence) = stmt.consequence() { self.scan_expression(&consequence); } - let post_if = self.scan.flow_state.snapshot(); - self.scan.flow_state.restore(pre_if); + let post_if = self.scan.bound_so_far.snapshot(); + self.scan.bound_so_far.restore(pre_if); if let Some(else_clause) = stmt.else_clause() { if let Ok(alternative) = else_clause.alternative() { @@ -204,7 +217,7 @@ impl SemanticIndexBuilder { } // Both branches' bindings are live afterwards. - self.scan.flow_state.merge(post_if); + self.scan.bound_so_far.merge(post_if); }, // `while`/`repeat` loops, subsets, extractions, parentheses, unary @@ -241,10 +254,11 @@ impl SemanticIndexBuilder { /// /// `Current + Eager` and `Nested + Eager` arguments are scanned here: /// `Current + Eager` transparently, `Nested + Eager` by descending into the - /// body and holding the names it binds as pending. `Nested + Lazy` and - /// `Current + Lazy` bodies are their own scan units and deferred to the walk - /// because resolution of effects in these lazy scopes needs the child's own - /// flow context. + /// body and staging the names it binds. A `Current + Lazy` body (`on_load()`) + /// is queued and scanned at the end of this scan unit's drain, once the + /// owner's bindings are complete. A `Nested + Lazy` body (`reactive()`) is + /// its own scan unit, deferred to the walk because resolution of effects in + /// that lazy scope needs the child's own flow context. fn scan_call(&mut self, call: &RCall) { let (arg_effects, attach, source, assign) = match self.resolve_effects(call) { Some(effects) => ( @@ -334,35 +348,47 @@ impl SemanticIndexBuilder { // Calls like `evalq()` (EvalEnv::Current, EvalTiming::Eager) => self.scan_expression(&value), - // Calls like `on_load()`. Its body runs later, so its defs - // land in the enclosing scope. We don't resolve the body's - // calls here. The walk does that once it enters the child - // scope. But we do grab the names it defines now, so the - // owner's bound names are complete before the walk reaches a sibling. + // Calls like `on_load()`. Its body runs later and binds + // into the owner scope, so we queue it and scan it at the + // end of this scan unit, once the owner's lexical + // environment is fully known. See `scan_deferred_bodies()`. (EvalEnv::Current, EvalTiming::Lazy) => { - self.record_enclosing_flow(value.syntax().text_trimmed_range()); - self.scan_lazy_owner_bindings(&value); + self.scan.deferred_bodies.push(DeferredBody { + body: value.clone(), + bound_so_far: self.scan.bound_so_far.snapshot(), + }); }, // Calls like `local()`. Its body runs eagerly at the call - // site, so its environment IS the current `flow_state`. - // Descend now, holding the names bound in this scope as - // pending so the walk has access to them. No `flow_state` - // reset: the child sees exactly what `begin_scan()` would - // have seeded. + // site, so its environment IS the current `bound_so_far`. + // Descend now, staging the names it binds as `Scanned` so the + // walk has access to them. No `bound_so_far` reset: the child + // sees exactly what `begin_scan()` would have seeded. // No `record_enclosing_flow()`: eager `Nested` bodies are // never scanned at walk time, so nothing would read it. (EvalEnv::Nested, EvalTiming::Eager) => { - let old = self.scan.flow_state.snapshot(); + let old = self.scan.bound_so_far.snapshot(); let range = value.syntax().text_trimmed_range(); - self.scan.eager_descent.open.push(BoundNames::new()); + let watermark = self.scan.deferred_bodies.len(); + self.scan.open_scopes.push(OpenScope { + kind: ScopeKind::Nse(EvalEnv::Nested, EvalTiming::Eager), + bindings: BindingSites::new(), + }); self.scan_expression(&value); - if let Some(bound) = self.scan.eager_descent.open.pop() { - self.scan.eager_descent.pending.insert(range, bound); + + // Drain `on_load`s inside this `local()` before popping + // its frame, so their names route to the `local` frame, + // not the scope below it. + self.scan_deferred_bodies(watermark); + + if let Some(scope) = self.scan.open_scopes.pop() { + self.scan + .body_scans + .insert(range, BodyScan::Scanned(scope.bindings)); } - self.scan.flow_state.restore(old); + self.scan.bound_so_far.restore(old); }, // Calls like `reactive()`. Its body runs at an unknown @@ -384,99 +410,37 @@ impl SemanticIndexBuilder { .arguments = Some(arg_effects); } - /// Copy the names a `Current + Lazy` body defines into the owner's - /// bound names, without marking them bound in the scan's flow state. - /// - /// This feeds enclosing snapshots only. A free variable elsewhere can - /// resolve to a name an `on_load({ ... })` defines in the owner, and - /// `register_enclosing_snapshot()` reads `bound_names` to find that ancestor. - /// The scan doesn't descend into these bodies otherwise, so their names - /// would only reach the owner when the walk later gets to the call, too late - /// for a sibling scanned before then. Collecting them now keeps the owner's - /// bound names complete before the walk touches any sibling. - /// - /// NSE shadow resolution does not read `bound_names`, so an incomplete - /// collection here can't flip an NSE decision. `is_locally_bound` reads the - /// captured eager bindings, which exclude deferred names by construction. - /// - /// We cover the realistic shapes, direct assignments and control flow, e.g. - /// `on_load({ x <- 1 })`. We stop at nested calls and function bodies - /// however, so we only add names the walk will also route, never a phantom. - /// `register_enclosing_snapshot` reads `binds()` as "a real definition - /// exists", so a phantom would send it chasing a binding that isn't there. - /// The price is a binding buried in a nested transparent call, e.g. - /// `on_load({ evalq(helper <- ...) })`, which we miss here, so a free - /// variable can't resolve to it. TODO(nse): We could potentially walk - /// transparent (Current) nested calls to collect those too. - /// - /// The names go to `bound_names` only, never to `flow_state`. The body - /// runs at some later time, so at an eager position after the call these - /// names aren't bound yet, and an eager callee there must still treat them - /// as unbound. - fn scan_lazy_owner_bindings(&mut self, expr: &AnyRExpression) { - match expr { - AnyRExpression::RBracedExpressions(braced) => { - for expr in braced.expressions().iter() { - self.scan_lazy_owner_bindings(&expr); - } - }, + /// Scan the `Current + Lazy` bodies queued since `watermark`, now that the + /// enclosing unit's `bound_anywhere` is complete. Runs with the owner's + /// frame context still live (the arena `current_scope`, plus any open eager + /// frame like a `local()` the `on_load` sits in). + pub(super) fn scan_deferred_bodies(&mut self, watermark: usize) { + // Take the queued bodies above `watermark` and scan them by value. + // Scanning a deferred body can enqueue nested `on_load()` bodies, which + // push past `watermark` again, so loop until the tail is empty. + // Splitting the tail off leaves the outer units' entries below + // `watermark` in place, and drops `deferred_bodies` back to `watermark` + // once drained. + loop { + let batch = self.scan.deferred_bodies.split_off(watermark); + if batch.is_empty() { + break; + } - AnyRExpression::RBinaryExpression(bin) => { - // `<<-` binds in an ancestor, not the owner, so it's not routed - // here (matching `add_definition`). - if !is_assignment(bin) || is_super_assignment(bin) { - return; - } - let target = if is_right_assignment(bin) { - bin.right() - } else { - bin.left() - }; - if let Ok(target) = target { - if let Some((name, range)) = assignment_name(&target) { - self.record_owner_name(name, range); - } - } - }, + for DeferredBody { body, bound_so_far } in batch { + let old = self.scan.bound_so_far.snapshot(); + self.scan.bound_so_far.restore(bound_so_far); - AnyRExpression::RIfStatement(stmt) => { - if let Ok(consequence) = stmt.consequence() { - self.scan_lazy_owner_bindings(&consequence); - } - if let Some(else_clause) = stmt.else_clause() { - if let Ok(alternative) = else_clause.alternative() { - self.scan_lazy_owner_bindings(&alternative); - } - } - }, + self.scan.open_scopes.push(OpenScope { + kind: ScopeKind::Nse(EvalEnv::Current, EvalTiming::Lazy), + bindings: BindingSites::new(), + }); - AnyRExpression::RForStatement(stmt) => { - if let Ok(variable) = stmt.variable() { - self.record_owner_name( - variable.name_text(), - variable.syntax().text_trimmed_range(), - ); - } - if let Ok(body) = stmt.body() { - self.scan_lazy_owner_bindings(&body); - } - }, + self.scan_expression(&body); - AnyRExpression::RWhileStatement(stmt) => { - if let Ok(body) = stmt.body() { - self.scan_lazy_owner_bindings(&body); - } - }, - - AnyRExpression::RRepeatStatement(stmt) => { - if let Ok(body) = stmt.body() { - self.scan_lazy_owner_bindings(&body); - } - }, - - // Stop everywhere else: function bodies are child scopes, and a - // call's arguments aren't part of this scope's direct level. - _ => {}, + self.scan.open_scopes.pop(); + self.scan.bound_so_far.restore(old); + } } } @@ -498,7 +462,7 @@ impl SemanticIndexBuilder { } pub(super) fn scan_parameter_defaults(&mut self, params: &RParameters) { - // Seed `flow_state` with every parameter names so a callee inside a + // Seed `bound_so_far` with every parameter names so a callee inside a // default value sees the full formal set for param in params.items().iter() { let Ok(param) = param else { continue }; @@ -508,7 +472,7 @@ impl SemanticIndexBuilder { AnyRParameterName::RDots(_) => String::from("..."), AnyRParameterName::RDotDotI(ddi) => ddi.syntax().text_trimmed().to_string(), }; - self.scan.flow_state.bind(text); + self.scan.bound_so_far.bind(text); } for param in params.items().iter() { @@ -564,30 +528,31 @@ impl SemanticIndexBuilder { /// [`scope_binds_anywhere`]: Self::scope_binds_anywhere fn scan_scope_binds(&self, name: &str) -> bool { match self.scan_scope() { - Some(ScanScope::Descent(bound)) => bound.binds(name), - Some(ScanScope::Scope(scope)) => self.scope_binds_anywhere(scope, name), - None => false, + ScanScope::Open(scope) => scope.binds(name), + ScanScope::Arena(scope) => self.scope_binds_anywhere(scope, name), } } fn scan_scope_is_global(&self) -> bool { match self.scan_scope() { - Some(ScanScope::Scope(scope)) => matches!(self.scopes[scope].kind, ScopeKind::File), - Some(ScanScope::Descent(_)) => false, - None => true, + ScanScope::Open(_) => false, + ScanScope::Arena(scope) => matches!(self.scopes[scope].kind, ScopeKind::File), } } - fn scan_scope(&self) -> Option> { - if let Some(bound) = self.scan.eager_descent.open.last() { - return Some(ScanScope::Descent(bound)); + fn scan_scope(&self) -> ScanScope<'_> { + // The current evaluation scope is the innermost open scope that owns + // its bindings. + for scope in self.scan.open_scopes.iter().rev() { + if scope.kind.owns_bindings() { + return ScanScope::Open(&scope.bindings); + } } - let scope = match self.scopes[self.current_scope].kind { - ScopeKind::Nse(EvalEnv::Current, EvalTiming::Lazy) => self.definition_owner()?, - _ => self.current_scope, - }; - Some(ScanScope::Scope(scope)) + // Return the arena's current scope if there is no owning open scope. + // This arena's scope is always owning because `Current + Lazy` bodies + // (e.g. `on_load()`) are scanned with their owner set to `current_scope`. + ScanScope::Arena(self.current_scope) } } @@ -596,49 +561,36 @@ impl SemanticIndexBuilder { impl SemanticIndexBuilder { /// Record the names a child scope (function body, NSE argument) about to be /// created at `range` inherits from its ancestors, to seed the child's scan - /// in `begin_scan`. Called during the scan, where `flow_state` is the + /// in `begin_scan`. Called during the scan, where `bound_so_far` is the /// parent's flow-precise state at the child's definition point (already /// carrying the parent's own inherited ancestors, so the child inherits /// transitively). fn record_enclosing_flow(&mut self, range: TextRange) { self.scan - .enclosing_flow - .insert(range, self.scan.flow_state.snapshot()); + .body_scans + .insert(range, BodyScan::Deferred(self.scan.bound_so_far.snapshot())); } - /// Record a binding in the scan's flow state. + /// Record a binding in both scan binding views. /// - /// The flow-precise `flow_state` always learns the name, so a - /// later callee in this scope sees it shadowed. The bound names only get it - /// when the current scope owns it. A `Current + Lazy` scope routes its defs - /// to the owner, so the name is added to the owner's bound names instead, the - /// same routing `add_definition_to_owner` does during the walk. + /// `bound_so_far` always learns the name, so a later eager callee in this + /// scope sees it shadowed. The name also routes into an owning scope's + /// `bound_anywhere`, matching `add_definition`'s routing during the walk. It + /// goes to the innermost open frame that owns its bindings (an eager + /// `Nested` body like `local()`); a lazy frame owns nothing, so its names + /// skip past it to the owner below. With no owning frame open, it lands in + /// the arena `current_scope`, which always owns its bindings at scan time + /// (the same invariant `scan_scope()` relies on). fn record_binding(&mut self, name: String, range: TextRange) { - self.record_owner_name(name.clone(), range); - self.scan.flow_state.bind(name); - } - - /// Route a binding NAME into its owner scope's bound names, matching - /// `add_definition`'s routing. When a descent is open the name goes to the - /// descent top, which is always an eager `Nested` body scanned inline and so - /// owns its bindings. Otherwise a `Current + Lazy` scope routes to - /// `definition_owner()` and every other scope owns its bindings. - /// - /// Split from `record_binding` so `scan_lazy_owner_bindings` can add - /// a deferred body's names to the owner's bound names without also marking them - /// bound in `flow_state` (see that helper for why). - fn record_owner_name(&mut self, name: String, range: TextRange) { - if let Some(bound) = self.scan.eager_descent.open.last_mut() { - bound.add(name, range); - return; - } + self.scan.bound_so_far.bind(name.clone()); - if let Some(target) = match self.scopes[self.current_scope].kind { - ScopeKind::Nse(EvalEnv::Current, EvalTiming::Lazy) => self.definition_owner(), - _ => Some(self.current_scope), - } { - self.scan.bound_names[target].add(name, range); + for frame in self.scan.open_scopes.iter_mut().rev() { + if frame.kind.owns_bindings() { + frame.bindings.add(name, range); + return; + } } + self.scan.bound_anywhere[self.current_scope].add(name, range); } } @@ -669,7 +621,7 @@ pub(super) struct SourcedFile { pub(super) resolution: Option, } -/// Backs a [`CallContext`]'s [`ScopeBindings`] with the builder's live scope +/// Backs a [`CallContext`]'s [`ScopeQuery`] with the builder's live scope /// state, so an effect handler (`substitute`) can query bindings during the /// scan without reaching into the builder directly. /// @@ -678,18 +630,18 @@ pub(super) struct ScanBindings<'a, R: ImportsResolver> { pub(super) builder: &'a SemanticIndexBuilder, } -impl ScopeBindings for ScanBindings<'_, R> { +impl ScopeContext for ScanBindings<'_, R> { fn is_bound(&self, name: &str, inherits: bool) -> bool { if inherits { - // The scan's `flow_state` carries the current scope's bindings plus + // The scan's `bound_so_far` carries the current scope's bindings plus // the inherited eager environment seeded at `begin_scan`, so it's // the lexical answer. - return self.builder.scan.flow_state.is_bound(name); + return self.builder.scan.bound_so_far.is_bound(name); } self.builder.scan_scope_binds(name) } - fn is_global_scope(&self) -> bool { + fn is_global(&self) -> bool { self.builder.scan_scope_is_global() } } @@ -741,47 +693,54 @@ impl FlowState { } } -/// Tracks eager `Nested` NSE bodies scanned inline during the scan. +/// A scope the scan has entered but the walk has not yet allocated in the +/// arena. A `local()` descent, or an `on_load()` body during its deferred scan. +/// Innermost last on the `open_scopes` stack. /// -/// An eager `Nested` body like `local()` runs immediately at its call site, so -/// we scan it inline instead of deferring it to the walk. `open` is the stack -/// of bodies being scanned right now, with the innermost on top. -/// `record_owner_name()` routes a binding to the top so names land on the body -/// that owns them. When a descent finishes, its names move to `pending`, keyed -/// by the body's range. +/// The arena scope doesn't exist yet because the walk allocates scopes in +/// preorder, and allocating one mid-scan would break the `Scope::descendants` +/// invariant. So a scope's names stage here on `bindings` while the scan is +/// inside it, keyed by nothing but stack position. /// -/// `pending` is keyed by range rather than written straight into -/// `bound_names[scope]` because the body's arena scope doesn't exist yet: the -/// walk allocates scopes in preorder, and allocating one mid-scan would break -/// the `Scope::descendants` invariant. The range is the body's pre-arena -/// identity until the walk pushes its scope. -/// -/// Once the walk pushes that scope, it installs the pending names into it -/// instead of re-scanning. It does this before collecting the body, because a -/// lazy child inside (a function or lazy NSE body) runs later than the walk -/// reaches it, so it can reference a binding defined further down this scope. -/// Resolving that name checks whether an enclosing scope binds it -/// (`scope_binds_anywhere()`), and the walk hasn't recorded that binding yet, so -/// the scan-populated bound set has to be complete up front. That's the reason -/// the scan collects bound names ahead of the walk at all. -#[derive(Default)] -pub(super) struct EagerNestedDescent { - pub(super) open: Vec, - pub(super) pending: FxHashMap, +/// `record_binding()` routes a binding to the innermost owning frame so names +/// land on the body that owns them. A `local()` descent finishes by moving its +/// `bindings` into `body_scans` as [`BodyScan::Scanned`], keyed by the body's +/// range, its pre-arena identity until the walk pushes its scope. An `on_load()` +/// body owns no names (they route to the owner), so its frame is discarded. +pub(super) struct OpenScope { + pub(super) kind: ScopeKind, + pub(super) bindings: BindingSites, +} + +/// Scan state for a child body, keyed by the body's text range (the body's +/// identity until the walk pushes its arena scope). +pub(super) enum BodyScan { + /// A walk-time scan unit (function body, `Nested + Lazy` like `reactive()`). + /// The walk seeds `begin_scan()` from this snapshot. + Deferred(FlowState), + /// Already scanned inline by an eager `Nested` descent (e.g. `local()`). + Scanned(BindingSites), +} + +/// A `Current + Lazy` body queued at its call site, scanned once its +/// enclosing scan unit finishes when the lexical environment is fully known. +#[derive(Clone)] +pub(super) struct DeferredBody { + pub(super) body: AnyRExpression, + /// `bound_so_far` captured at the call site, the body's inherited eager env. + pub(super) bound_so_far: FlowState, } /// All definitions in a scope, collected by the scan pass before the /// walk. Skips child-scope bodies (nested functions and `Nested` NSE bodies). /// -/// Keeps each name's earliest binding site in scan order, which is source -/// order within the scope. A name bound several times reads as bound -/// throughout, and this earliest site is what the lazy-shadow diagnostic points -/// at, once `is_lazily_shadowed` has picked the nearest binding ancestor. -pub(super) struct BoundNames { +/// Keeps each name's earliest binding site in scan order. This earliest site is +/// mentioned in the lazy-shadow diagnostics. +pub(super) struct BindingSites { by_name: FxHashMap, } -impl BoundNames { +impl BindingSites { pub(super) fn new() -> Self { Self { by_name: FxHashMap::default(), @@ -802,12 +761,12 @@ impl BoundNames { } /// A scope as the scan sees it. A `local()` body scanned inline has no arena -/// scope yet and its bindings are stored in the staging [`EagerNestedDescent`]. +/// scope yet and its bindings are stored on the [`ScanScope`] `open` stack. /// Every other scope is materialized in the arena. [`scan_scope`] resolves /// which one is the current evaluation frame. /// /// [`scan_scope`]: SemanticIndexBuilder::scan_scope enum ScanScope<'a> { - Descent(&'a BoundNames), - Scope(ScopeId), + Open(&'a BindingSites), + Arena(ScopeId), } diff --git a/crates/oak_semantic/src/builder/walk.rs b/crates/oak_semantic/src/builder/walk.rs index 26f561a59..7fce1620f 100644 --- a/crates/oak_semantic/src/builder/walk.rs +++ b/crates/oak_semantic/src/builder/walk.rs @@ -28,6 +28,7 @@ use super::assignment_name; use super::is_assignment; use super::is_right_assignment; use super::is_super_assignment; +use super::scan::BodyScan; use super::scan::SourcedFile; use super::SemanticIndexBuilder; use crate::effects::AssignBinding; @@ -308,13 +309,14 @@ impl SemanticIndexBuilder { } fn walk_function(&mut self, fun: &RFunctionDefinition) { + let watermark = self.scan.deferred_bodies.len(); let scope = self.push_scope(ScopeKind::Function, fun.syntax().text_trimmed_range()); if let Ok(params) = fun.parameters() { // Scan the default values before collecting them. R binds all // formals into the frame at once, so a default sees every parameter // name regardless of position: `function(local, b = local(...))` is - // not NSE. So we seed the whole formal set into `flow_state` + // not NSE. So we seed the whole formal set into `bound_so_far` // up front rather than flow-ordered, then scan each default. self.begin_scan(); self.scan_parameter_defaults(¶ms); @@ -324,9 +326,11 @@ impl SemanticIndexBuilder { // above recorded. self.walk_parameters(¶ms); } + if let Ok(body) = fun.body() { self.begin_scan(); self.scan_expression(&body); + self.scan_deferred_bodies(watermark); self.walk_expression(&body); } @@ -640,11 +644,6 @@ impl SemanticIndexBuilder { } /// Walk a single NSE argument body, pushing a scope when appropriate. - /// - /// `Current + Eager` stays in the current scope. `Nested + Eager` was - /// already scanned by the descent, so we install its pending names and only - /// walk. The remaining lazy bodies are their own scan units that we scan - /// here on entry. fn walk_nse_argument(&mut self, env: EvalEnv, timing: EvalTiming, value: &AnyRExpression) { match (env, timing) { // Calls like `evalq()` @@ -658,21 +657,21 @@ impl SemanticIndexBuilder { let kind = ScopeKind::Nse(EvalEnv::Nested, EvalTiming::Eager); let scope = self.push_scope(kind, range); - // Install the pending names the descent recorded for this body, + // Install the scanned names the descent recorded for this body, // before collecting so lazy children inside can see them via // `scope_binds_anywhere()`. - match self.scan.eager_descent.pending.remove(&range) { - Some(bound) => self.scan.bound_names[scope] = bound, - None => { + match self.scan.body_scans.remove(&range) { + Some(BodyScan::Scanned(bound)) => self.scan.bound_anywhere[scope] = bound, + _ => { // An eager NSE scope is reachable only through the scan - // unit that descended into it, so the pending set must + // unit that descended into it, so its scanned entry must // exist. If not this is a builder bug. In release // builds we still scan the body here so the walk can // proceed. This fallback runs with an empty eager // environment and its shadow decisions are more // degraded than a real lazy unit's. stdext::debug_panic!( - "Missing pending bound names for eager NSE body at {range:?}" + "Missing scanned bound names for eager NSE body at {range:?}" ); self.begin_scan(); self.scan_expression(value); @@ -683,16 +682,27 @@ impl SemanticIndexBuilder { self.pop_scope(scope); }, - (env, timing) => { - let kind = ScopeKind::Nse(env, timing); + // Calls like `on_load()`. The deferred drain already scanned this + // body (its names are in the owner, its NSE decisions cached), so we + // only push the scope and walk. + (EvalEnv::Current, EvalTiming::Lazy) => { + let kind = ScopeKind::Nse(EvalEnv::Current, EvalTiming::Lazy); + let scope = self.push_scope(kind, value.syntax().text_trimmed_range()); + self.walk_expression(value); + self.pop_scope(scope); + }, + + // Calls like `reactive()`. Its own scan unit, scanned here on entry + // in the child's flow context. Its body can queue `on_load()`s that + // own the `reactive()` scope, so drain them before walking. + (EvalEnv::Nested, EvalTiming::Lazy) => { + let kind = ScopeKind::Nse(EvalEnv::Nested, EvalTiming::Lazy); let scope = self.push_scope(kind, value.syntax().text_trimmed_range()); - // Scan the child body before walking it. A `Current + Lazy` - // scope routes its defs to the owner and holds no bound names of its - // own, which `record_binding` handles; the scan still runs to - // record the body's NSE decisions in the child's flow context. self.begin_scan(); + let watermark = self.scan.deferred_bodies.len(); self.scan_expression(value); + self.scan_deferred_bodies(watermark); self.walk_expression(value); self.pop_scope(scope); }, @@ -717,13 +727,10 @@ impl SemanticIndexBuilder { kind: DefinitionKind, range: TextRange, ) { - // `Nse(Current, Lazy)` scopes don't own any definitions. We add the - // definitions to the real enclosing owner scope. Note that `Current + - // Eager` never reaches here because it doesn't push a scope. - if matches!( - self.scopes[self.current_scope].kind, - ScopeKind::Nse(EvalEnv::Current, EvalTiming::Lazy) - ) { + // A scope that doesn't own its bindings routes its definitions to the + // enclosing owner scope. Only `Nse(Current, Lazy)` (`on_load`) reaches + // here as such: `Current + Eager` pushes no scope, so it never does. + if !self.scopes[self.current_scope].kind.owns_bindings() { self.add_definition_to_owner(name, flags, kind, range); return; } @@ -750,19 +757,19 @@ impl SemanticIndexBuilder { kind: DefinitionKind, range: TextRange, ) { - let Some(target_scope) = self.definition_owner() else { + let Some(owner_scope) = self.enclosing_owner() else { stdext::debug_panic!("Current + Lazy scope has no parent"); return; }; - let symbol_id = self.walk.symbol_tables[target_scope].intern(name, flags); - let def_id = self.walk.definitions[target_scope].push(Definition { + let symbol_id = self.walk.symbol_tables[owner_scope].intern(name, flags); + let def_id = self.walk.definitions[owner_scope].push(Definition { symbol: symbol_id, kind, range, }); - self.walk.use_def_maps[target_scope].ensure_symbol(symbol_id); + self.walk.use_def_maps[owner_scope].ensure_symbol(symbol_id); // Deferred: the body executes at an unknown later time, so the // definition shouldn't shadow what's already live. This is the same @@ -773,7 +780,7 @@ impl SemanticIndexBuilder { // file-level uses that run before the lazy body executes. Ideally // these defs would only be reachable from lazy scopes (functions), // not from eager/file-level code. - self.walk.use_def_maps[target_scope].record_deferred_definition(symbol_id, def_id); + self.walk.use_def_maps[owner_scope].record_deferred_definition(symbol_id, def_id); } // Super-assignment is lexically in the current scope but binds in an diff --git a/crates/oak_semantic/src/effects.rs b/crates/oak_semantic/src/effects.rs index f2ef12bdf..f643cd6df 100644 --- a/crates/oak_semantic/src/effects.rs +++ b/crates/oak_semantic/src/effects.rs @@ -123,7 +123,7 @@ pub trait AssignHandler: std::fmt::Debug + Sync { /// in the current scope (so they resolve here, against substitute's env) from /// those that stay quoted (so they resolve wherever the result is later /// evaluated). -pub trait ScopeBindings { +pub trait ScopeContext { /// Whether `name` is bound in the current scope. With `inherits`, also /// counts bindings inherited from enclosing scopes, mirroring R's /// `get(..., inherits=)`. @@ -132,7 +132,7 @@ pub trait ScopeBindings { /// Whether the current scope is the global (file) scope. R's `substitute` /// substitutes nothing in the global environment, so a handler falls back to /// a plain quote there. - fn is_global_scope(&self) -> bool; + fn is_global(&self) -> bool; } /// Whether an assign effect reads its target before writing it. @@ -151,34 +151,33 @@ pub enum TargetAccess { /// binding state of the surrounding scope. #[derive(Default)] pub struct CallContext<'a> { - bindings: Option<&'a dyn ScopeBindings>, + scope: Option<&'a dyn ScopeContext>, } impl<'a> CallContext<'a> { /// A context backed by the builder's scope state, for handlers that query /// bindings (`substitute`). - pub fn with_bindings(bindings: &'a dyn ScopeBindings) -> Self { + pub fn with_bindings(bindings: &'a dyn ScopeContext) -> Self { Self { - bindings: Some(bindings), + scope: Some(bindings), } } /// Whether `name` is bound in the current scope (see - /// [`ScopeBindings::is_bound`]). Without a bindings backing (a [`Default`] + /// [`ScopeQuery::is_bound`]). Without a bindings backing (a [`Default`] /// context) we can't tell, so we answer "unbound", the choice that leaves a /// symbol quoted rather than treating it as a use. pub fn is_bound(&self, name: &str, inherits: bool) -> bool { - self.bindings - .is_some_and(|bindings| bindings.is_bound(name, inherits)) + self.scope + .is_some_and(|scope| scope.is_bound(name, inherits)) } /// Whether the current scope is the global (file) scope (see - /// [`ScopeBindings::is_global_scope`]). Without a bindings backing (a + /// [`ScopeQuery::is_global_scope`]). Without a bindings backing (a /// [`Default`] context) we assume global, so `substitute` degrades to a /// plain quote (its no-substitution behaviour). pub fn current_scope_is_global(&self) -> bool { - self.bindings - .is_none_or(|bindings| bindings.is_global_scope()) + self.scope.is_none_or(|scope| scope.is_global()) } /// Match `call`'s arguments to `formals`, returning for each call argument diff --git a/crates/oak_semantic/src/semantic_index.rs b/crates/oak_semantic/src/semantic_index.rs index 15bf9f555..338fe5e57 100644 --- a/crates/oak_semantic/src/semantic_index.rs +++ b/crates/oak_semantic/src/semantic_index.rs @@ -498,6 +498,17 @@ impl ScopeKind { ScopeKind::Nse(_, timing) => timing == EvalTiming::Lazy, } } + + /// Whether this scope is its own evaluation environment, holding its own + /// bindings. `Current` NSE scopes (`evalq()`, `rlang::on_load()`) share + /// their bindings with a parent scope. All other scopes are owners. + /// + /// Orthogonal to [`is_lazy`](Self::is_lazy), which is about timing: + /// `shiny::reactive()` owns its bindings and is lazy, `base::local()` owns + /// its bindings and is eager. + pub fn owns_bindings(self) -> bool { + !matches!(self, ScopeKind::Nse(EvalEnv::Current, _)) + } } impl Scope { diff --git a/crates/oak_semantic/tests/integration/contrib/rlang.rs b/crates/oak_semantic/tests/integration/contrib/rlang.rs index 3e8f3347b..d6790c2f4 100644 --- a/crates/oak_semantic/tests/integration/contrib/rlang.rs +++ b/crates/oak_semantic/tests/integration/contrib/rlang.rs @@ -259,15 +259,16 @@ local({ x <- 1 }) #[test] fn test_nse_descent_current_lazy_owner_routes_to_descent_top() { // A `Current + Lazy` body (`on_load`) inside an eager `local` body binds `x`. - // During the descent, `record_owner_name` must route `x` to the descent top - // (local), not to the current scope. `scan_lazy_owner_bindings` runs while - // the arena's `current_scope` is still the file, so only the descent-top - // shortcut lands `x` in local's pending names. + // During the descent, `record_binding` must route `x` to the descent top + // (local), not to the current scope. The `on_load` body is scanned in the + // deferred drain while `local`'s frame is still open, so its assignment lands + // `x` in local's bound names. // // We pin it through a FORWARD reference: `f` uses `x` before `on_load` binds - // it, so the walk resolves the use through local's `bound_names` (the pending - // set), not through an already-recorded definition. If the routing regressed, - // `x` would land in the file and the use would resolve to the file scope. + // it, so the walk resolves the use through local's `bound_anywhere` (the + // scanned set), not through an already-recorded definition. If the routing + // regressed, `x` would land in the file and the use would resolve to the + // file scope. let index = index( "\ local({ @@ -288,3 +289,56 @@ local({ let (enclosing_scope, _bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, local_scope); } + +#[test] +fn test_nse_descent_current_lazy_owner_routes_assign_to_descent_top() { + // Same as above, but the `on_load` body binds `x` through `assign("x", 1)` + // instead of `<-`. The deferred scan runs the full scan machinery over the + // body, so it recognizes the `assign()` effect and routes `x` to `local`, + // the descent top. The old reduced preview stopped at nested calls and never + // saw the `assign`, so `f`'s forward use of `x` could not resolve to it. + let index = index( + "\ +local({ + f <- function() x + rlang::on_load({ assign(\"x\", 1) }) +}) +", + ); + let local_scope = ScopeId::from(1); + let f_scope = ScopeId::from(2); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(EvalEnv::Nested, EvalTiming::Eager) + ); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + + let (enclosing_scope, _bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); + assert_eq!(enclosing_scope, local_scope); +} + +#[test] +fn test_nse_descent_current_lazy_owner_routes_delayed_assign_to_descent_top() { + // The `delayedAssign("x", ...)` variant of the `assign()` case above, another + // call-based binding the full deferred scan recognizes and routes to `local`. + let index = index( + "\ +local({ + f <- function() x + rlang::on_load({ delayedAssign(\"x\", 1) }) +}) +", + ); + let local_scope = ScopeId::from(1); + let f_scope = ScopeId::from(2); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(EvalEnv::Nested, EvalTiming::Eager) + ); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + + let (enclosing_scope, _bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); + assert_eq!(enclosing_scope, local_scope); +}