Skip to content

Commit

Permalink
Remove FileDescriptor::Unknown and revert PathResolver to work on…
Browse files Browse the repository at this point in the history
… strings

Strings for the `PathResolver` represent user paths, which is the only context
in which import path resolution makes sense.
  • Loading branch information
ggiraldez committed Oct 11, 2024
1 parent 40ace53 commit fc623fb
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 60 deletions.
6 changes: 3 additions & 3 deletions crates/metaslang/bindings/generated/public_api.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 28 additions & 10 deletions crates/metaslang/bindings/src/builder/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,30 @@ mod resolver {
let path_to_resolve = parameters.param()?.into_string()?;
parameters.finish()?;

let context_path_kind = FileDescriptor::from_string(&context_path);
let context_file_descriptor = FileDescriptor::from_string(&context_path);
let Ok(FileDescriptor::User(context_user_path)) = context_file_descriptor else {
// Since the path resolver should only map to user paths from
// user paths, it is an error to attempt to resolve a path in
// the context of a system file
return Err(ExecutionError::FunctionFailed(
"resolve-path".into(),
"Cannot execute with a non-user context file path".into(),
));
};

let resolved_path = self
.path_resolver
.as_ref()
.resolve_path(&context_path_kind, &path_to_resolve)
.as_string_or_else(|| {
// In case we cannot resolve the path, we return a special value that is unique
// per context/path pait. This way, we can still run incrementally and resolve
// other symbols in the file:
format!("__SLANG_UNRESOLVED_PATH__{context_path}__{path_to_resolve}__")
});
.resolve_path(&context_user_path, &path_to_resolve)
.map_or_else(
|| {
// In case we cannot resolve the path, we return a special value that is unique
// per context/path pait. This way, we can still run incrementally and resolve
// other symbols in the file:
format!("__SLANG_UNRESOLVED_PATH__{context_path}__{path_to_resolve}__")
},
|resolved_path| FileDescriptor::User(resolved_path).as_string(),
);

Ok(resolved_path.into())
}
Expand All @@ -114,8 +127,13 @@ mod resolver {
let file_path = parameters.param()?.into_string()?;
parameters.finish()?;

let is_system_file = FileDescriptor::from_string(&file_path).is_system();
Ok(is_system_file.into())
let Ok(file_descriptor) = FileDescriptor::from_string(&file_path) else {
return Err(ExecutionError::FunctionFailed(
"is-system-file".into(),
"Parameter is not a valid file path".into(),
));
};
Ok(file_descriptor.is_system().into())
}
}
}
45 changes: 22 additions & 23 deletions crates/metaslang/bindings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,40 +56,37 @@ pub struct Bindings<KT: KindTypes + 'static> {
}

pub enum FileDescriptor {
Unknown,
User(String),
System(String),
}

pub(crate) struct FileDescriptorError;

impl FileDescriptor {
// Internal functions to convert a FileKind to a string for representation inside the stack graph
pub(crate) fn as_string(&self) -> String {
self.as_string_or_else(|| unreachable!("cannot convert an Unknown file kind to string"))
}
// Internal functions to convert a FileDescriptor to and from a string for
// representation inside the stack graph

pub(crate) fn as_string_or_else(&self, f: impl FnOnce() -> String) -> String {
pub(crate) fn as_string(&self) -> String {
match self {
Self::User(path) => format!("user:{path}"),
Self::System(path) => format!("system:{path}"),
Self::Unknown => f(),
}
}

pub(crate) fn from_string(value: &str) -> Self {
pub(crate) fn from_string(value: &str) -> Result<Self, FileDescriptorError> {
if let Some(path) = value.strip_prefix("user:") {
FileDescriptor::User(path.into())
Ok(FileDescriptor::User(path.into()))
} else if let Some(path) = value.strip_prefix("system:") {
FileDescriptor::System(path.into())
Ok(FileDescriptor::System(path.into()))
} else {
FileDescriptor::Unknown
Err(FileDescriptorError)
}
}

pub fn get_path(&self) -> Option<&str> {
pub fn get_path(&self) -> &str {
match self {
Self::User(path) => Some(path),
Self::System(path) => Some(path),
Self::Unknown => None,
Self::User(path) => path,
Self::System(path) => path,
}
}

Expand All @@ -100,10 +97,14 @@ impl FileDescriptor {
pub fn is_user(&self) -> bool {
matches!(self, Self::User(_))
}

pub fn is_user_path(&self, path: &str) -> bool {
matches!(self, Self::User(user_path) if user_path == path)
}
}

pub trait PathResolver {
fn resolve_path(&self, context_path: &FileDescriptor, path_to_resolve: &str) -> FileDescriptor;
fn resolve_path(&self, context_path: &str, path_to_resolve: &str) -> Option<String>;
}

impl<KT: KindTypes + 'static> Bindings<KT> {
Expand Down Expand Up @@ -343,7 +344,7 @@ impl<'a, KT: KindTypes + 'static> fmt::Display for DisplayCursor<'a, KT> {
f,
"`{}` at {}:{}:{}",
self.cursor.node().unparse(),
self.file.get_path().unwrap_or("<unknown_file>"),
self.file.get_path(),
offset.line + 1,
offset.column + 1,
)
Expand Down Expand Up @@ -371,9 +372,8 @@ impl<'a, KT: KindTypes + 'static> Definition<'a, KT> {
pub fn get_file(&self) -> FileDescriptor {
self.owner.stack_graph[self.handle]
.file()
.map_or(FileDescriptor::Unknown, |file| {
FileDescriptor::from_string(self.owner.stack_graph[file].name())
})
.and_then(|file| FileDescriptor::from_string(self.owner.stack_graph[file].name()).ok())
.unwrap_or_else(|| unreachable!("Definition does not have a valid file descriptor"))
}

pub(crate) fn has_tag(&self, tag: Tag) -> bool {
Expand Down Expand Up @@ -459,9 +459,8 @@ impl<'a, KT: KindTypes + 'static> Reference<'a, KT> {
pub fn get_file(&self) -> FileDescriptor {
self.owner.stack_graph[self.handle]
.file()
.map_or(FileDescriptor::Unknown, |file| {
FileDescriptor::from_string(self.owner.stack_graph[file].name())
})
.and_then(|file| FileDescriptor::from_string(self.owner.stack_graph[file].name()).ok())
.unwrap_or_else(|| unreachable!("Reference does not have a valid file descriptor"))
}

pub fn jump_to_definition(&self) -> Result<Definition<'a, KT>, ResolutionError<'a, KT>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'a> fmt::Display for DefinitionAssertion<'a> {
f,
"Assert Definition {id} {cursor}",
id = self.id,
cursor = DisplayCursor(&self.cursor, Some(self.file)),
cursor = DisplayCursor(&self.cursor, self.file),
)
}
}
Expand All @@ -103,11 +103,11 @@ impl<'a> fmt::Display for ReferenceAssertion<'a> {
if let Some(version_req) = &self.version_req {
write!(f, " in versions {version_req}")?;
}
write!(f, " {}", DisplayCursor(&self.cursor, Some(self.file)))
write!(f, " {}", DisplayCursor(&self.cursor, self.file))
}
}

struct DisplayCursor<'a>(&'a Cursor, Option<&'a str>);
struct DisplayCursor<'a>(&'a Cursor, &'a str);

impl<'a> fmt::Display for DisplayCursor<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -116,7 +116,7 @@ impl<'a> fmt::Display for DisplayCursor<'a> {
f,
"`{}` at {}:{}:{}",
self.0.node().unparse(),
self.1.unwrap_or("<unknown_file>"),
self.1,
offset.line + 1,
offset.column + 1,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ pub(crate) fn render_bindings(
buffer.push(b'\n');
}

let part_references = bindings.all_references().filter(|reference| {
let ref_file = reference.get_file();
ref_file.is_user() && ref_file.get_path().unwrap() == part.path
});
let part_references = bindings
.all_references()
.filter(|reference| reference.get_file().is_user_path(part.path));
let (bindings_report, part_all_resolved) =
build_report_for_part(part, &all_definitions, part_references);
all_resolved = all_resolved && part_all_resolved;
Expand Down Expand Up @@ -90,8 +89,7 @@ fn build_report_for_part<'a>(
let Some(cursor) = definition.get_cursor() else {
continue;
};
let def_file = definition.get_file();
if !def_file.is_user() || def_file.get_path().unwrap() != part.path {
if !definition.get_file().is_user_path(part.path) {
continue;
}

Expand Down
10 changes: 3 additions & 7 deletions crates/solidity/outputs/cargo/tests/src/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
use metaslang_bindings::{FileDescriptor, PathResolver};
use metaslang_bindings::PathResolver;

pub struct TestsPathResolver;

impl PathResolver for TestsPathResolver {
fn resolve_path(&self, context_path: &FileDescriptor, path_to_resolve: &str) -> FileDescriptor {
let FileDescriptor::User(context_path) = context_path else {
return FileDescriptor::Unknown;
};
fn resolve_path(&self, context_path: &str, path_to_resolve: &str) -> Option<String> {
if is_relative_path(path_to_resolve) {
// Relative import: the actual path will be computed using the
// context path (ie. the path of the importing source unit)
normalize_path(path_to_resolve, get_parent_path(context_path))
.map_or(FileDescriptor::Unknown, FileDescriptor::User)
} else {
// Direct import: this path will be used as-is
FileDescriptor::User(path_to_resolve.to_string())
Some(path_to_resolve.to_string())
}
}
}
Expand Down
10 changes: 3 additions & 7 deletions crates/solidity/testing/perf/benches/iai/tests/init_bindings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use metaslang_bindings::{FileDescriptor, PathResolver};
use metaslang_bindings::PathResolver;
use slang_solidity::bindings::{create_with_resolver, get_built_ins, Bindings};
use slang_solidity::parser::Parser;

Expand All @@ -22,11 +22,7 @@ pub fn run() -> Bindings {
struct NoOpResolver;

impl PathResolver for NoOpResolver {
fn resolve_path(
&self,
_context_path: &FileDescriptor,
path_to_resolve: &str,
) -> FileDescriptor {
FileDescriptor::User(path_to_resolve.to_string())
fn resolve_path(&self, _context_path: &str, path_to_resolve: &str) -> Option<String> {
Some(path_to_resolve.to_string())
}
}

0 comments on commit fc623fb

Please sign in to comment.