Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions crates/oak_semantic/src/builder/builder_nse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ impl<R: ImportsResolver> SemanticIndexBuilder<R> {

match arg_effects[i] {
None => self.scan_expression(&value),
// Quoted argument: captured unevaluated. No effects to scan, no
// names to bind.
Some(Argument {
effect: ArgumentEffect::Quote,
..
}) => {},
Some(Argument {
effect: ArgumentEffect::Nse { scope, timing },
..
Expand Down Expand Up @@ -506,9 +512,16 @@ impl<R: ImportsResolver> SemanticIndexBuilder<R> {
let Ok(arg) = item else { continue };
let Some(value) = arg.value() else { continue };

match arg_effects[i] {
None => self.collect_expression(&value),
Some(argument) => self.collect_nse_argument(argument, &value),
let Some(argument) = arg_effects[i] else {
self.collect_expression(&value);
continue;
};
match argument.effect {
ArgumentEffect::Nse { scope, timing } => {
self.collect_nse_argument(scope, timing, &value)
},
// Quoted argument: No uses inside.
ArgumentEffect::Quote => {},
}
}
}
Expand All @@ -519,8 +532,7 @@ impl<R: ImportsResolver> SemanticIndexBuilder<R> {
/// 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 collect_nse_argument(&mut self, argument: &Argument, value: &AnyRExpression) {
let ArgumentEffect::Nse { scope, timing } = argument.effect;
fn collect_nse_argument(&mut self, scope: NseScope, timing: NseTiming, value: &AnyRExpression) {
match (scope, timing) {
// Calls like `evalq()`
(NseScope::Current, NseTiming::Eager) => {
Expand Down
4 changes: 4 additions & 0 deletions crates/oak_semantic/src/effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ pub struct Argument {
pub enum ArgumentEffect {
/// Quote plus Eval in a controlled scope, fused
Nse { scope: NseScope, timing: NseTiming },
/// Captured unevaluated, so its symbols are not uses and nothing in it runs.
/// `quote`, `bquote`. TODO:`bquote(.(foo))` should unquote `foo`. This
/// requires implementing the `Eval` effect.
Quote,
}

impl EffectHandler for ArgumentsAnnotation {
Expand Down
27 changes: 27 additions & 0 deletions crates/oak_semantic/src/effects_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,30 @@ macro_rules! nse {
};
}

/// A quoted entry. Each `(name, position)` names an argument captured
/// unevaluated: its symbols aren't uses and nothing in it runs. `quote`,
/// `bquote`.
macro_rules! quoted {
($pkg:literal, $func:literal, $(($name:literal, $pos:literal)),+ $(,)?) => {
Entry {
package: $pkg,
function: $func,
effects: EffectsHandlers {
arguments: Some(&ArgumentsAnnotation {
arguments: &[$(Argument {
name: $name,
position: $pos,
effect: ArgumentEffect::Quote,
}),+],
}),
attach: None,
source: None,
assign: None,
},
}
};
}

/// An attach entry: `(package-argument position, has-`character.only`-flag)`.
macro_rules! attach {
($pkg:literal, $func:literal, $pos:literal, $character_only:literal) => {
Expand Down Expand Up @@ -120,6 +144,9 @@ static REGISTRY: &[Entry] = &[
nse!("base", "with.default", ("expr", 1, Nested, Eager)),
nse!("base", "within", ("expr", 1, Nested, Eager)),
nse!("base", "within.data.frame", ("expr", 1, Nested, Eager)),
// base quote
quoted!("base", "quote", ("expr", 0)),
quoted!("base", "bquote", ("expr", 0)),
// base attach
attach!("base", "library", 0, true),
attach!("base", "require", 0, true),
Expand Down
88 changes: 74 additions & 14 deletions crates/oak_semantic/tests/integration/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,9 +859,9 @@ fn test_super_assignment_with_use_on_value_side() {

// --- NSE / quoting constructs ---
//
// Identifiers inside `~`, `quote()`, and `bquote()` are currently recorded
// as uses. This is a known simplification; refining it is deferred as
// future work. These tests document the current behaviour.
// `quote()` and `bquote()` capture their argument unevaluated, so the symbols
// inside are not uses (the tests below). Identifiers inside a formula `~` are
// still recorded as uses, a known simplification left for future work.

#[test]
fn test_fixme_formula_records_uses() {
Expand Down Expand Up @@ -898,32 +898,92 @@ fn test_fixme_one_sided_formula_records_uses() {
}

#[test]
fn test_fixme_quote_records_uses() {
let index = index("quote(x + y)");
fn test_quote_suppresses_uses() {
// `quote()` captures its argument unevaluated, so the symbols inside are not
// uses. Only the callee `quote` itself is a use.
let index = index_with_base("quote(x + y)");
let file = ScopeId::from(0);

assert_eq!(
index.symbols(file).get("quote").unwrap().flags(),
SymbolFlags::IS_USED
);
assert_eq!(
index.symbols(file).get("x").unwrap().flags(),
SymbolFlags::IS_USED
);
assert!(index.symbols(file).get("x").is_none());
assert!(index.symbols(file).get("y").is_none());
assert_eq!(index.uses(file).len(), 1);
}

#[test]
fn test_quote_suppresses_assignment() {
// The `x <- 1` inside `quote()` is captured unevaluated, so it binds
// nothing.
let index = index_with_base("quote(x <- 1)");
let file = ScopeId::from(0);

assert!(index.symbols(file).get("x").is_none());
assert_eq!(index.definitions(file).len(), 0);
}

#[test]
fn test_bquote_suppresses_uses() {
// `bquote()` quotes its argument the same as `quote()`.
let index = index_with_base("bquote(x + y)");
let file = ScopeId::from(0);

assert!(index.symbols(file).get("x").is_none());
assert!(index.symbols(file).get("y").is_none());
assert_eq!(index.uses(file).len(), 1);
}

#[test]
fn test_fixme_bquote_unquote_hole_suppressed() {
// TODO(eval): `bquote`'s `.()` holes are evaluated, so `y` here is really a
// use. We treat the whole argument as quoted until the `Eval` effect lands,
// so `y` is suppressed for now.
let index = index_with_base("bquote(x + .(y))");
let file = ScopeId::from(0);

assert!(index.symbols(file).get("y").is_none());
}

#[test]
fn test_local_quote_definition_shadows() {
// A local `quote` shadows base's, so `quote(y)` is an ordinary call and `y`
// is a use again.
let index = index_with_base("quote <- function(a) a\nquote(y)");
let file = ScopeId::from(0);

assert_eq!(
index.symbols(file).get("y").unwrap().flags(),
SymbolFlags::IS_USED
);
}

#[test]
fn test_fixme_quote_records_assignment() {
let index = index("quote(x <- 1)");
fn test_quote_suppresses_attach_effect() {
// A `library()` inside `quote()` is captured unevaluated, so it attaches
// nothing to the search path.
let index = index_with_base("quote(library(dplyr))");
assert!(semantic_call_kinds(&index).is_empty());
}

#[test]
fn test_quote_suppresses_source_effect() {
// A `source()` inside `quote()` is captured unevaluated, so it injects no
// names and records no source call.
let index = index_with_base("quote(source(\"helpers.R\"))");
assert!(semantic_call_kinds(&index).is_empty());
}

#[test]
fn test_quote_suppresses_assign_effect() {
// An `assign()` inside `quote()` is captured unevaluated, so it binds
// nothing.
let index = index_with_base("quote(assign(\"x\", 1))");
let file = ScopeId::from(0);

let x = index.symbols(file).get("x").unwrap();
assert_eq!(x.flags(), SymbolFlags::IS_BOUND);
assert_eq!(index.definitions(file).len(), 1);
assert!(index.symbols(file).get("x").is_none());
assert_eq!(index.definitions(file).len(), 0);
}

#[test]
Expand Down
Loading