Skip to content

Commit

Permalink
added validation for generic params of impl functions (#6934)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TomerStarkware
TomerStarkware authored Dec 30, 2024
1 parent e133839 commit f593c6d
Show file tree
Hide file tree
Showing 4 changed files with 352 additions and 5 deletions.
57 changes: 56 additions & 1 deletion crates/cairo-lang-semantic/src/diagnostic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fmt::Display;
use cairo_lang_debug::DebugWithDb;
use cairo_lang_defs::diagnostic_utils::StableLocation;
use cairo_lang_defs::ids::{
EnumId, FunctionTitleId, ImplDefId, ImplFunctionId, ModuleId, ModuleItemId,
EnumId, FunctionTitleId, GenericKind, ImplDefId, ImplFunctionId, ModuleId, ModuleItemId,
NamedLanguageElementId, StructId, TopLevelLanguageElementId, TraitFunctionId, TraitId,
TraitImplId, UseId,
};
Expand Down Expand Up @@ -337,6 +337,47 @@ impl DiagnosticEntry for SemanticDiagnostic {
actual_ty.format(db)
)
}

SemanticDiagnosticKind::WrongGenericParamTraitForImplFunction {
impl_def_id,
impl_function_id,
trait_id,
expected_trait,
actual_trait,
} => {
let defs_db = db.upcast();
let function_name = impl_function_id.name(defs_db);
format!(
"Generic parameter trait of impl function `{}::{}` is incompatible with \
`{}::{}`. Expected: `{:?}`, actual: `{:?}`.",
impl_def_id.name(defs_db),
function_name,
trait_id.name(defs_db),
function_name,
expected_trait.debug(db),
actual_trait.debug(db)
)
}
SemanticDiagnosticKind::WrongGenericParamKindForImplFunction {
impl_def_id,
impl_function_id,
trait_id,
expected_kind,
actual_kind,
} => {
let defs_db = db.upcast();
let function_name = impl_function_id.name(defs_db);
format!(
"Generic parameter kind of impl function `{}::{}` is incompatible with \
`{}::{}`. Expected: `{:?}`, actual: `{:?}`.",
impl_def_id.name(defs_db),
function_name,
trait_id.name(defs_db),
function_name,
expected_kind,
actual_kind
)
}
SemanticDiagnosticKind::AmbiguousTrait { trait_function_id0, trait_function_id1 } => {
format!(
"Ambiguous method call. More than one applicable trait function with a \
Expand Down Expand Up @@ -1070,6 +1111,20 @@ pub enum SemanticDiagnosticKind {
trait_id: TraitId,
expected_name: SmolStr,
},
WrongGenericParamTraitForImplFunction {
impl_def_id: ImplDefId,
impl_function_id: ImplFunctionId,
trait_id: TraitId,
expected_trait: ConcreteTraitId,
actual_trait: ConcreteTraitId,
},
WrongGenericParamKindForImplFunction {
impl_def_id: ImplDefId,
impl_function_id: ImplFunctionId,
trait_id: TraitId,
expected_kind: GenericKind,
actual_kind: GenericKind,
},
WrongType {
expected_ty: semantic::TypeId,
actual_ty: semantic::TypeId,
Expand Down
88 changes: 84 additions & 4 deletions crates/cairo-lang-semantic/src/items/imp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3146,10 +3146,91 @@ fn validate_impl_function_signature(
);
return Ok(trait_function_id);
}
let substitution =
let impl_def_substitution = db.impl_def_substitution(impl_def_id)?;
let func_generics: Vec<GenericParam> =
SubstitutionRewriter { db, substitution: impl_def_substitution.as_ref() }
.rewrite(func_generics)?;

let function_substitution =
GenericSubstitution::new(&func_generics, &generic_params_to_args(impl_func_generics, db));
let concrete_trait_signature = SubstitutionRewriter { db, substitution: &substitution }
.rewrite(concrete_trait_signature)?;

for (trait_generic_param, generic_param) in izip!(func_generics, impl_func_generics.iter()) {
if let Some(name) = trait_generic_param.id().name(defs_db) {
if Some(name.clone()) != generic_param.id().name(defs_db) {
diagnostics.report(generic_param.stable_ptr(defs_db), WrongParameterName {
impl_def_id,
impl_function_id,
trait_id,
expected_name: name,
});
}
}
match (generic_param, trait_generic_param) {
(GenericParam::Type(_), GenericParam::Type(_)) => {}
(GenericParam::Impl(generic_param), GenericParam::Impl(trait_generic_param))
| (GenericParam::NegImpl(generic_param), GenericParam::NegImpl(trait_generic_param)) => {
let rewritten_trait_param_trait =
SubstitutionRewriter { db, substitution: &function_substitution }
.rewrite(trait_generic_param.concrete_trait)?;
let rewritten_trait_param_type_constraints =
SubstitutionRewriter { db, substitution: &function_substitution }
.rewrite(trait_generic_param.type_constraints)?;
generic_param
.concrete_trait
.map(|actual_trait| {
rewritten_trait_param_trait
.map(|expected_trait| {
if actual_trait != expected_trait
|| generic_param.type_constraints
!= rewritten_trait_param_type_constraints
{
diagnostics.report(
generic_param.id.stable_ptr(defs_db),
WrongGenericParamTraitForImplFunction {
impl_def_id,
impl_function_id,
trait_id,
expected_trait,
actual_trait,
},
);
}
})
.ok();
})
.ok();
}
(GenericParam::Const(generic_param), GenericParam::Const(trait_generic_param)) => {
let expected_ty = SubstitutionRewriter { db, substitution: &function_substitution }
.rewrite(trait_generic_param.ty)?;
if generic_param.ty != expected_ty {
diagnostics.report(generic_param.id.stable_ptr(defs_db), WrongParameterType {
impl_def_id,
impl_function_id,
trait_id,
expected_ty,
actual_ty: generic_param.ty,
});
}
}
(generic_param, trait_generic_param) => {
diagnostics.report(
generic_param.stable_ptr(defs_db),
WrongGenericParamKindForImplFunction {
impl_def_id,
impl_function_id,
trait_id,
expected_kind: trait_generic_param.kind(),
actual_kind: generic_param.kind(),
},
);
}
}
}

let concrete_trait_signature =
SubstitutionRewriter { db, substitution: &function_substitution }
.rewrite(concrete_trait_signature)?;

if signature.params.len() != concrete_trait_signature.params.len() {
diagnostics.report(&signature_syntax.parameters(syntax_db), WrongNumberOfParameters {
Expand All @@ -3160,7 +3241,6 @@ fn validate_impl_function_signature(
actual: signature.params.len(),
});
}
let impl_def_substitution = db.impl_def_substitution(impl_def_id)?;
let concrete_trait_signature =
SubstitutionRewriter { db, substitution: impl_def_substitution.as_ref() }
.rewrite(concrete_trait_signature)?;
Expand Down
181 changes: 181 additions & 0 deletions crates/cairo-lang-semantic/src/items/tests/trait
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1327,3 +1327,184 @@ error: Cycle detected while resolving 'impls alias' items.
--> lib.cairo:3:6
impl Ifelt = I0<Ifelt>;
^^^^^

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

//! > impl function with wrong generic arg name.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
fn foo() -> felt252 {
2
}

//! > function_name
foo

//! > module_code
trait Tr {
fn foo<T>(x: T);
}

impl I of Tr {
fn foo<U>(x: U) {}
}

//! > expected_diagnostics
error: Parameter name of impl function I::foo is incompatible with Tr::foo parameter `T`.
--> lib.cairo:6:12
fn foo<U>(x: U) {}
^

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

//! > impl function with wrong generic arg trait.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
fn foo() -> felt252 {
2
}

//! > function_name
foo

//! > module_code
trait Tr {
fn foo<T, +Destruct<T>>(x: T);
}

impl I of Tr {
fn foo<T, +Drop<T>>(x: T) {}
}

//! > expected_diagnostics
error: Generic parameter trait of impl function `I::foo` is incompatible with `Tr::foo`. Expected: `core::traits::Destruct::<T>`, actual: `core::traits::Drop::<T>`.
--> lib.cairo:6:15
fn foo<T, +Drop<T>>(x: T) {}
^^^^^^^^

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

//! > impl function with the wrong generic arg concrete trait.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
fn foo() -> felt252 {
2
}

//! > function_name
foo

//! > module_code
trait Tr {
fn foo<T, S, +Destruct<T>>(x: T);
}

impl I of Tr {
fn foo<T, S, +Destruct<S>>(x: T) {}
}

//! > expected_diagnostics
error: Generic parameter trait of impl function `I::foo` is incompatible with `Tr::foo`. Expected: `core::traits::Destruct::<T>`, actual: `core::traits::Destruct::<S>`.
--> lib.cairo:6:18
fn foo<T, S, +Destruct<S>>(x: T) {}
^^^^^^^^^^^^

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

//! > impl function with the same generic arg trait.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: false)

//! > function
fn foo() -> felt252 {
2
}

//! > function_name
foo

//! > module_code
trait Tr {
fn foo<T, +Destruct<T>>(x: T);
}

impl I of Tr {
fn foo<T, +Destruct<T>>(x: T) {}
}

//! > expected_diagnostics

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

//! > impl function with the wrong generic arg kind.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
fn foo() -> felt252 {
2
}

//! > function_name
foo

//! > module_code
trait Tr {
fn foo<T, +Destruct<T>>();
}

impl I of Tr {
fn foo<const T: i32, S>() {}
}

//! > expected_diagnostics
error: Generic parameter kind of impl function `I::foo` is incompatible with `Tr::foo`. Expected: `Type`, actual: `Const`.
--> lib.cairo:6:12
fn foo<const T: i32, S>() {}
^^^^^^^^^^^^

error: Generic parameter kind of impl function `I::foo` is incompatible with `Tr::foo`. Expected: `Impl`, actual: `Type`.
--> lib.cairo:6:26
fn foo<const T: i32, S>() {}
^

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

//! > impl function with the wrong const generic kind.

//! > test_runner_name
test_function_diagnostics(expect_diagnostics: true)

//! > function
fn foo() -> felt252 {
2
}

//! > function_name
foo

//! > module_code
trait Tr {
fn foo<T, const C: T>();
}

impl I of Tr {
fn foo<T, const C: i32>() {}
}

//! > expected_diagnostics
error: Parameter type of impl function `I::foo` is incompatible with `Tr::foo`. Expected: `T`, actual: `core::integer::i32`.
--> lib.cairo:6:15
fn foo<T, const C: i32>() {}
^^^^^^^^^^^^
Loading

0 comments on commit f593c6d

Please sign in to comment.