Skip to content

Commit

Permalink
Fail on mismatch in method return types for ABIs and traits (FuelLabs…
Browse files Browse the repository at this point in the history
…#3022)

Replace unification with equality for return types. Unification does not
make a distinction between some integer types.

ref FuelLabs#2397


*Edit*:
I think I need to check parameter types as well and if needed do them in
a separate PR to keep it simpler and reduce review times.
  • Loading branch information
anton-trunov authored Oct 13, 2022
1 parent aa6d4ef commit f8ff054
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 20 deletions.
30 changes: 15 additions & 15 deletions sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::{HashMap, HashSet};

use sway_error::error::{CompileError, InterfaceName};
use sway_types::{Ident, Span, Spanned};

use crate::{
Expand All @@ -14,7 +13,7 @@ use crate::{
insert_type, look_up_type_id, set_type_as_storage_only, to_typeinfo, unify_with_self,
CopyTypes, TypeId, TypeMapping, TypeParameter,
},
CompileResult, TyFunctionDeclaration, TypeInfo,
CompileError, CompileResult, TyFunctionDeclaration, TypeInfo,
};

use super::TyTraitFn;
Expand Down Expand Up @@ -493,6 +492,7 @@ fn type_check_trait_implementation(
block_span: &Span,
is_contract: bool,
) -> CompileResult<Vec<TyFunctionDeclaration>> {
use sway_error::error::InterfaceName;
let interface_name = || -> InterfaceName {
if is_contract {
InterfaceName::Abi(trait_name.suffix.clone())
Expand Down Expand Up @@ -596,7 +596,8 @@ fn type_check_trait_implementation(
);
warnings.append(&mut new_warnings);
if !new_errors.is_empty() {
errors.push(CompileError::MismatchedTypeInTrait {
errors.push(CompileError::MismatchedTypeInInterfaceSurface {
interface_name: interface_name(),
span: fn_decl_param.type_span.clone(),
given: fn_decl_param_type.to_string(),
expected: fn_signature_param_type.to_string(),
Expand Down Expand Up @@ -625,18 +626,17 @@ fn type_check_trait_implementation(
});
}

// unify the return type of the function declaration
// with the return type of the function signature
let (mut new_warnings, new_errors) = unify_with_self(
fn_decl.return_type,
fn_signature.return_type,
ctx.self_type(),
&fn_decl.return_type_span,
ctx.help_text(),
);
warnings.append(&mut new_warnings);
if !new_errors.is_empty() {
errors.push(CompileError::MismatchedTypeInTrait {
// the return type of the function declaration must be the same
// as the return type of the function signature
let self_type = ctx.self_type();
use super::ReplaceSelfType;
let mut fn_decl_ret_type = fn_decl.return_type;
fn_decl_ret_type.replace_self_type(self_type);
let mut fn_sign_ret_type = fn_signature.return_type;
fn_sign_ret_type.replace_self_type(self_type);
if look_up_type_id(fn_decl_ret_type) != look_up_type_id(fn_sign_ret_type) {
errors.push(CompileError::MismatchedTypeInInterfaceSurface {
interface_name: interface_name(),
span: fn_decl.return_type_span.clone(),
expected: fn_signature.return_type.to_string(),
given: fn_decl.return_type.to_string(),
Expand Down
7 changes: 4 additions & 3 deletions sway-error/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,10 @@ pub enum CompileError {
#[error(
"Expected: {expected} \n\
found: {given}. The definition of this function must \
match the one in the trait declaration."
match the one in the {interface_name} declaration."
)]
MismatchedTypeInTrait {
MismatchedTypeInInterfaceSurface {
interface_name: InterfaceName,
span: Span,
given: String,
expected: String,
Expand Down Expand Up @@ -694,7 +695,7 @@ impl Spanned for CompileError {
AssociatedFunctionCalledAsMethod { span, .. } => span.clone(),
TypeParameterNotInTypeScope { span, .. } => span.clone(),
MultipleImmediates(span) => span.clone(),
MismatchedTypeInTrait { span, .. } => span.clone(),
MismatchedTypeInInterfaceSurface { span, .. } => span.clone(),
NotATrait { span, .. } => span.clone(),
UnknownTrait { span, .. } => span.clone(),
FunctionNotAPartOfInterfaceSurface { span, .. } => span.clone(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,19 @@ contract;

abi MyContract {
fn foo(x: u64) -> str[7];
fn bar() -> u32;
fn baz() -> u64;
}

impl MyContract for Contract {
fn foo(s: str[7]) -> str[7] {
s
}

fn bar() -> u64 {
0
}

fn baz() { // No return type here
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,12 @@ category = "fail"

# check: fn foo(s: str[7]) -> str[7] {
# nextln: $()Expected: u64
# nextln: $()found: str[7]. The definition of this function must match the one in the trait declaration.
# nextln: $()found: str[7]. The definition of this function must match the one in the ABI "MyContract" declaration.

# check: fn bar() -> u64 {
# nextln: $()Expected: u32
# nextln: $()found: u64. The definition of this function must match the one in the ABI "MyContract" declaration.

# check: fn baz() {
# nextln: $()Expected: u64
# nextln: $()found: (). The definition of this function must match the one in the ABI "MyContract" declaration.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ library test;
trait Foo {
fn foo(x: u64) -> str[7];
fn bar(variable: u64) -> bool;
fn baz() -> u32;
fn quux() -> u64;
}

struct S {
Expand All @@ -20,4 +22,11 @@ impl Foo for S {
fn bar(ref mut variable: u64) -> bool {
true
}

fn baz() -> u64 {
0
}

fn quux() { // no return type
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@ category = "fail"

# check: fn foo(s: str[7]) -> str[7] {
# nextln: $()Expected: u64
# nextln: $()found: str[7]. The definition of this function must match the one in the trait declaration.
# nextln: $()found: str[7]. The definition of this function must match the one in the trait "Foo" declaration.

# check: fn bar(ref mut variable: u64) -> bool {
# nextln: $()Parameter mutability mismatch between the trait function declaration and its implementation.

# check: fn baz() -> u64 {
# nextln: $()Expected: u32
# nextln: $()found: u64. The definition of this function must match the one in the trait "Foo" declaration.

# check: fn quux() {
# nextln: $()Expected: u64
# nextln: $()found: (). The definition of this function must match the one in the trait "Foo" declaration.

0 comments on commit f8ff054

Please sign in to comment.