Skip to content

Add external symbol resolution infrastructure#1151

Merged
lionel- merged 4 commits intomainfrom
oak/external-resolution
Apr 17, 2026
Merged

Add external symbol resolution infrastructure#1151
lionel- merged 4 commits intomainfrom
oak/external-resolution

Conversation

@lionel-
Copy link
Copy Markdown
Contributor

@lionel- lionel- commented Apr 14, 2026

Branched from #1150
Part of #1148

  • Adds data structures for the scope chains of binding sources
  • The semantic index now tracks library() and require() calls and creates scope chains from them.
  • New helpers to resolve unbound symbols from external definitions (scope chains).

For future work:

  • Create scope chains for files in a package, respecting the collation order (aaa.R will have a larger chain than zzz.R).
  • Resolve symbol at point in a file from the current scope chain tracked in the index (e.g. library() calls), potentially concatenated to the global scope chain.

Comment thread crates/oak_index/src/external.rs Outdated

/// Walk the scope chain front-to-back, returning the first match.
pub fn resolve_external_name(
library: &Library,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some weird gut feeling about library being an external input to the system

Like it feels like BindingSource::PackageExports should already have all the info it needs to resolve the symbol name

I don't feel strongly enough to request a change, just pointing out something that smells kinda maybe off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep how we handle library is definitely up in the air

Comment on lines +141 to +147
fn test_resolve_first_match_wins() {
let library = test_library(vec![("stats", vec!["filter"])]);

let scope = vec![
file_exports("utils.R", vec![("filter", range(0, 6))]),
BindingSource::PackageExports("stats".to_string()),
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also a test for two packages that have bindings that shadow each other?

I think that last package wins in that case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep last one wins

@lionel- lionel- force-pushed the oak/package-crate-2 branch 2 times, most recently from 3f6a202 to ddad93d Compare April 17, 2026 15:24
Base automatically changed from oak/package-crate-2 to main April 17, 2026 15:32
@lionel- lionel- force-pushed the oak/external-resolution branch from 8475c57 to bec6eda Compare April 17, 2026 15:52
@lionel- lionel- mentioned this pull request Apr 17, 2026
3 tasks
@lionel- lionel- merged commit 9168963 into main Apr 17, 2026
12 of 13 checks passed
@lionel- lionel- deleted the oak/external-resolution branch April 17, 2026 15:59
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants