Skip to content

Commit

Permalink
Raise diagnostic on impl inference cycle. (#4367)
Browse files Browse the repository at this point in the history
  • Loading branch information
gilbens-starkware authored Nov 6, 2023
1 parent 56faba3 commit 5878fcf
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 7 deletions.
2 changes: 2 additions & 0 deletions crates/cairo-lang-semantic/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ pub trait SemanticGroup:
// ==============
/// Returns candidate [ImplDefId]s for a specific trait lookup constraint.
#[salsa::invoke(items::imp::module_impl_ids_for_trait_filter)]
#[salsa::cycle(items::imp::module_impl_ids_for_trait_filter_cycle)]
fn module_impl_ids_for_trait_filter(
&self,
module_id: ModuleId,
Expand Down Expand Up @@ -497,6 +498,7 @@ pub trait SemanticGroup:
fn impl_def_resolver_data(&self, impl_def_id: ImplDefId) -> Maybe<Arc<ResolverData>>;
/// Returns the concrete trait that is implemented by the impl.
#[salsa::invoke(items::imp::impl_def_concrete_trait)]
#[salsa::cycle(items::imp::impl_def_concrete_trait_cycle)]
fn impl_def_concrete_trait(&self, impl_def_id: ImplDefId) -> Maybe<ConcreteTraitId>;
/// Returns the attributes attached to the impl.
#[salsa::invoke(items::imp::impl_def_attributes)]
Expand Down
4 changes: 3 additions & 1 deletion crates/cairo-lang-semantic/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ impl DiagnosticEntry for SemanticDiagnostic {
"Cycle detected while resolving 'impls alias' items.".into()
}
SemanticDiagnosticKind::ImplRequirementCycle => {
"Cycle detected while resolving generic param.".into()
"Cycle detected while resolving generic param. Try specifying the generic impl \
parameter explicitly to break the cycle."
.into()
}
SemanticDiagnosticKind::MissingMember { member_name } => {
format!(r#"Missing member "{member_name}"."#)
Expand Down
34 changes: 34 additions & 0 deletions crates/cairo-lang-semantic/src/expr/test_data/generics
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,37 @@ error: Trait not found.
--> lib.cairo:1:16
fn bar<impl X: unknown>() {}
^*****^

//! > ==========================================================================

//! > Test impl infernce cycle

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
trait Introspect<T> {
}

#[derive(Drop)]
struct Generic<T, +Introspect<T>> {
value: T,
}

fn foo() {
let generic = Generic { value: 123 };
}

impl GenericStructIntrospect<T, impl IntrospectTImp: Introspect<T>> of Introspect<Generic<T>> {}
impl Felt252Introspect of Introspect<felt252> {}

//! > function_name
foo

//! > module_code

//! > expected_diagnostics
error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
--> lib.cairo:13:72
impl GenericStructIntrospect<T, impl IntrospectTImp: Introspect<T>> of Introspect<Generic<T>> {}
^********************^
33 changes: 29 additions & 4 deletions crates/cairo-lang-semantic/src/items/imp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,17 @@ pub fn impl_def_concrete_trait(
db.priv_impl_declaration_data(impl_def_id)?.concrete_trait
}

/// Cycle handling for [crate::db::SemanticGroup::impl_def_concrete_trait].
pub fn impl_def_concrete_trait_cycle(
_db: &dyn SemanticGroup,
_cycle: &[String],
_impl_def_id: &ImplDefId,
) -> Maybe<ConcreteTraitId> {
// The diagnostics will be reported from the calling function, specifically from
// `priv_impl_declaration_data_inner`.
Err(skip_diagnostic())
}

/// Query implementation of [crate::db::SemanticGroup::impl_def_attributes].
pub fn impl_def_attributes(
db: &dyn SemanticGroup,
Expand Down Expand Up @@ -364,10 +375,10 @@ pub fn priv_impl_declaration_data_inner(
.and_then(|concrete_item| {
try_extract_matches!(concrete_item, ResolvedConcreteItem::Trait)
})
.ok_or_else(|| diagnostics.report(&trait_path_syntax, NotATrait))
} else {
None
}
.ok_or_else(|| diagnostics.report(&trait_path_syntax, NotATrait));
Err(diagnostics.report(&trait_path_syntax, ImplRequirementCycle))
};

// Check fully resolved.
if let Some((stable_ptr, inference_err)) = resolver.inference().finalize() {
Expand Down Expand Up @@ -719,7 +730,9 @@ pub fn module_impl_ids_for_trait_filter(
if trait_id != trait_filter.trait_id {
continue;
}
let Ok(concrete_trait_id) = uninferred_impl.concrete_trait(db) else { continue };
let Ok(concrete_trait_id) = uninferred_impl.concrete_trait(db) else {
continue;
};
if let Ok(true) = concrete_trait_fits_trait_filter(db, concrete_trait_id, &trait_filter) {
res.push(uninferred_impl);
}
Expand All @@ -728,6 +741,18 @@ pub fn module_impl_ids_for_trait_filter(
Ok(res)
}

/// Cycle handeling for [crate::db::SemanticGroup::module_impl_ids_for_trait_filter].
pub fn module_impl_ids_for_trait_filter_cycle(
_db: &dyn SemanticGroup,
_cycle: &[String],
_module_id: &ModuleId,
_trait_filter: &TraitFilter,
) -> Maybe<Vec<UninferredImpl>> {
// The diagnostics will be reported from the calling function, specifically from
// `priv_impl_declaration_data_inner`.
Err(skip_diagnostic())
}

/// Checks whether an [ImplDefId] passes a [TraitFilter].
fn concrete_trait_fits_trait_filter(
db: &dyn SemanticGroup,
Expand Down
2 changes: 1 addition & 1 deletion crates/cairo-lang-semantic/src/items/tests/module
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ error: The name `abc` is defined multiple times.
impl abc of abc;
^*^

error: Not a trait.
error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
--> lib.cairo:22:13
impl abc of abc;
^*^
44 changes: 43 additions & 1 deletion crates/cairo-lang-semantic/src/items/tests/trait
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ error: Return type of impl function `MyImpl2::no_ret_ty` is incompatible with `M
fn no_ret_ty(ref a: u128) {
^

error: Not a trait.
error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
--> lib.cairo:34:13
impl abc of abc;
^*^
Expand Down Expand Up @@ -619,3 +619,45 @@ error: The number of parameters in the impl function `MyImpl::foo` is incompatib
--> lib.cairo:7:14
fn foo<S>(a: felt252, b: S, c: felt252) {}
^**************************^

//! > ==========================================================================

//! > Test impl of an impl instead of trait.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
fn bla() {
}

//! > function_name
bla

//! > module_code
trait t1 {}
impl imp1 of t1 {}
impl imp2 of imp1 {}
impl imp3 of imp3 {}
mod inner {
trait t_inner {}
impl imp of t_inner {}
}
use inner::imp as inner_imp;
impl imp4 of inner_imp {}

//! > expected_diagnostics
error: Not a trait.
--> lib.cairo:3:14
impl imp2 of imp1 {}
^**^

error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
--> lib.cairo:4:14
impl imp3 of imp3 {}
^**^

error: Not a trait.
--> lib.cairo:10:14
impl imp4 of inner_imp {}
^*******^

0 comments on commit 5878fcf

Please sign in to comment.