Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disambiguate field or int static trait method call #6112

Merged
merged 6 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
53 changes: 41 additions & 12 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,12 @@
interned_statement_kinds: noirc_arena::Arena<StatementKind>,

// Interned `UnresolvedTypeData`s during comptime code.
interned_unresolved_type_datas: noirc_arena::Arena<UnresolvedTypeData>,

Check warning on line 224 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (datas)

// Interned `Pattern`s during comptime code.
interned_patterns: noirc_arena::Arena<Pattern>,

/// Determins whether to run in LSP mode. In LSP mode references are tracked.

Check warning on line 229 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Determins)
pub(crate) lsp_mode: bool,

/// Store the location of the references in the graph.
Expand Down Expand Up @@ -361,7 +361,16 @@
#[derive(Default, Debug, Clone)]
pub struct Methods {
pub direct: Vec<FuncId>,
pub trait_impl_methods: Vec<FuncId>,
pub trait_impl_methods: Vec<TraitImplMethod>,
}

#[derive(Debug, Clone)]
pub struct TraitImplMethod {
// This type is only stored for primitive types to be able to
// select the correct static methods between multiple options keyed
// under TypeMethodKey::FieldOrInt
pub typ: Option<Type>,
pub method: FuncId,
}

/// All the information from a function that is filled out during definition collection rather than
Expand Down Expand Up @@ -656,7 +665,7 @@
quoted_types: Default::default(),
interned_expression_kinds: Default::default(),
interned_statement_kinds: Default::default(),
interned_unresolved_type_datas: Default::default(),

Check warning on line 668 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (datas)
interned_patterns: Default::default(),
lsp_mode: false,
location_indices: LocationIndices::default(),
Expand Down Expand Up @@ -1383,7 +1392,7 @@
.or_default()
.entry(method_name)
.or_default()
.add_method(method_id, is_trait_method);
.add_method(method_id, None, is_trait_method);
None
}
Type::Error => None,
Expand All @@ -1395,12 +1404,16 @@
let key = get_type_method_key(self_type).unwrap_or_else(|| {
unreachable!("Cannot add a method to the unsupported type '{}'", other)
});
// Only remember the actual type if it's FieldOrInt,
// so later we can disambiguate on calls like `u32::call`.
let typ =
if key == TypeMethodKey::FieldOrInt { Some(self_type.clone()) } else { None };
self.primitive_methods
.entry(key)
.or_default()
.entry(method_name)
.or_default()
.add_method(method_id, is_trait_method);
.add_method(method_id, typ, is_trait_method);
None
}
}
Expand Down Expand Up @@ -2131,11 +2144,11 @@
&mut self,
typ: UnresolvedTypeData,
) -> InternedUnresolvedTypeData {
InternedUnresolvedTypeData(self.interned_unresolved_type_datas.insert(typ))

Check warning on line 2147 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (datas)
}

pub fn get_unresolved_type_data(&self, id: InternedUnresolvedTypeData) -> &UnresolvedTypeData {
&self.interned_unresolved_type_datas[id.0]

Check warning on line 2151 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (datas)
}

/// Returns the type of an operator (which is always a function), along with its return type.
Expand Down Expand Up @@ -2246,23 +2259,29 @@
if self.direct.len() == 1 {
Some(self.direct[0])
} else if self.direct.is_empty() && self.trait_impl_methods.len() == 1 {
Some(self.trait_impl_methods[0])
Some(self.trait_impl_methods[0].method)
} else {
None
}
}

fn add_method(&mut self, method: FuncId, is_trait_method: bool) {
fn add_method(&mut self, method: FuncId, typ: Option<Type>, is_trait_method: bool) {
if is_trait_method {
self.trait_impl_methods.push(method);
let trait_impl_method = TraitImplMethod { typ, method };
self.trait_impl_methods.push(trait_impl_method);
} else {
self.direct.push(method);
}
}

/// Iterate through each method, starting with the direct methods
pub fn iter(&self) -> impl Iterator<Item = FuncId> + '_ {
self.direct.iter().copied().chain(self.trait_impl_methods.iter().copied())
pub fn iter(&self) -> impl Iterator<Item = (FuncId, &Option<Type>)> + '_ {
let trait_impl_methods = self.trait_impl_methods.iter().map(|m| (m.method, &m.typ));
let direct = self.direct.iter().copied().map(|func_id| {
let typ: &Option<Type> = &None;
(func_id, typ)
});
direct.chain(trait_impl_methods)
}

/// Select the 1 matching method with an object type matching `typ`
Expand All @@ -2274,7 +2293,7 @@
) -> Option<FuncId> {
// When adding methods we always check they do not overlap, so there should be
// at most 1 matching method in this list.
for method in self.iter() {
for (method, method_type) in self.iter() {
match interner.function_meta(&method).typ.instantiate(interner).0 {
Type::Function(args, _, _, _) => {
if has_self_param {
Expand All @@ -2287,15 +2306,25 @@
}
}
} else {
// Just return the first method whose name matches since we
// can't match object types on static methods.
return Some(method);
// If we recorded the concrete type this trait impl method belongs to,
// and it matches typ, it's an exact match and we return that.
if let Some(method_type) = method_type {
let mut bindings = TypeBindings::new();

if method_type.try_unify(typ, &mut bindings).is_ok() {
Type::apply_type_bindings(bindings);
return Some(method);
}
} else {
return Some(method);
}
}
}
Type::Error => (),
other => unreachable!("Expected function type, found {other}"),
}
}

None
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "field_or_int_static_trait_method"
type = "bin"
authors = [""]
compiler_version = ">=0.32.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
trait Read {
asterite marked this conversation as resolved.
Show resolved Hide resolved
fn read(data: [Field; 1]) -> Self;
}

impl Read for Field {
fn read(data: [Field; 1]) -> Self {
data[0] * 10
}
}

impl Read for u32 {
fn read(data: [Field; 1]) -> Self {
data[0] as u32
}
}

fn main() {
let data = [1];

let value: u32 = u32::read(data);
assert_eq(value, 1);

let value: Field = Field::read(data);
assert_eq(value, 10);
}
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
use super::process_request;

mod auto_import;
mod builtins;

Check warning on line 44 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (builtins)
mod completion_items;
mod kinds;
mod sort_text;
Expand Down Expand Up @@ -235,7 +235,7 @@
let mut idents: Vec<Ident> = Vec::new();

// Find in which ident we are in, and in which part of it
// (it could be that we are completting in the middle of an ident)

Check warning on line 238 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (completting)
for segment in &path.segments {
let ident = &segment.ident;

Expand Down Expand Up @@ -627,7 +627,7 @@
};

for (name, methods) in methods_by_name {
for func_id in methods.iter() {
for (func_id, _method_type) in methods.iter() {
if name_matches(name, prefix) {
let completion_items = self.function_completion_items(
name,
Expand Down Expand Up @@ -1645,8 +1645,8 @@
}

true
}

Check warning on line 1648 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (merk)

Check warning on line 1649 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (merk)
fn module_def_id_from_reference_id(reference_id: ReferenceId) -> Option<ModuleDefId> {
match reference_id {
ReferenceId::Module(module_id) => Some(ModuleDefId::ModuleId(module_id)),
Expand Down
Loading