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

Respect variable type ascription when initializer has type Never #6911

Merged
merged 13 commits into from
Feb 12, 2025
2 changes: 1 addition & 1 deletion sway-core/src/language/ty/declaration/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ impl TyDecl {
let type_engine = engines.te();
let decl_engine = engines.de();
let type_id = match self {
TyDecl::VariableDecl(decl) => decl.body.return_type,
TyDecl::VariableDecl(decl) => decl.return_type,
TyDecl::FunctionDecl(FunctionDecl { decl_id, .. }) => {
let decl = decl_engine.get_function(decl_id);
decl.return_type.type_id
Expand Down
51 changes: 29 additions & 22 deletions sway-core/src/semantic_analysis/ast_node/declaration/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,37 +49,44 @@ impl ty::TyVariableDecl {
None,
)
.unwrap_or_else(|err| type_engine.id_of_error_recovery(err));

let mut ctx = ctx
.with_type_annotation(type_ascription.type_id)
.with_help_text(
"Variable declaration's type annotation does not match up \
with the assigned expression's type.",
with the assigned expression's type.",
);

let result = ty::TyExpression::type_check(handler, ctx.by_ref(), &var_decl.body);
let body = result
.unwrap_or_else(|err| ty::TyExpression::error(err, var_decl.name.span(), engines));

// TODO: Integers shouldn't be anything special. RHS expressions should be written in
// a way to always use the context provided from the LHS, and if the LHS is
// an integer, RHS should properly unify or type check should fail.
// Remove this special case as a part of the initiative of improving type inference.
// Integers are special in the sense that we can't only rely on the type of `body`
// to get the type of the variable. The type of the variable *has* to follow
// `type_ascription` if `type_ascription` is a concrete integer type that does not
// conflict with the type of `body` (i.e. passes the type checking above).
let return_type = match &*type_engine.get(type_ascription.type_id) {
TypeInfo::UnsignedInteger(_) => type_ascription.type_id,
_ => match &*type_engine.get(body.return_type) {
// If RHS type check ends up in an error we want to use the
// provided type ascription as the variable type. E.g.:
// let v: Struct<u8> = Struct<u64> { x: 0 }; // `v` should be "Struct<u8>".
// let v: ExistingType = non_existing_identifier; // `v` should be "ExistingType".
// let v = <some error>; // `v` will remain "{unknown}".
// TODO: Refine and improve this further. E.g.,
// let v: Struct { /* MISSING FIELDS */ }; // Despite the error, `v` should be of type "Struct".
TypeInfo::ErrorRecovery(_) => type_ascription.type_id,
_ => body.return_type,
},
// Determine the type of the variable going forward. Typically this is the type of the RHS,
// but in some cases we need to use the type ascription instead.
// TODO: We should not have these special cases. The typecheck expressions should be written
// in a way to always use the context provided by the LHS, and use the unified type of LHS
// and RHS as the return type of the RHS. Remove this special case as a part of the
// initiative of improving type inference.
let return_type = match (&*type_engine.get(type_ascription.type_id), &*type_engine.get(body.return_type)) {
// Integers: We can't rely on the type of the RHS to give us the correct integer
// type, so the type of the variable *has* to follow `type_ascription` if
// `type_ascription` is a concrete integer type that does not conflict with the type
// of `body` (i.e. passes the type checking above).
(TypeInfo::UnsignedInteger(_), _) |
// Never: If the RHS resolves to Never, then any code following the declaration is
// unreachable. If the variable is used later on, then it should be treated as
// having the same type as the type ascription.
(_, TypeInfo::Never) |
// If RHS type check ends up in an error we want to use the
// provided type ascription as the variable type. E.g.:
// let v: Struct<u8> = Struct<u64> { x: 0 }; // `v` should be "Struct<u8>".
// let v: ExistingType = non_existing_identifier; // `v` should be "ExistingType".
// let v = <some error>; // `v` will remain "{unknown}".
// TODO: Refine and improve this further. E.g.,
// let v: Struct { /* MISSING FIELDS */ }; // Despite the error, `v` should be of type "Struct".
(_, TypeInfo::ErrorRecovery(_)) => type_ascription.type_id,
// In all other cases we use the type of the RHS.
_ => body.return_type,
};

if !ctx.code_block_first_pass() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2427,7 +2427,7 @@ impl ty::TyExpression {
));
}

break (name, variable_decl.body.return_type);
break (name, variable_decl.return_type);
}
TyDecl::ConstantDecl(constant_decl) => {
let constant_decl =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ fn diverge_in_match_condition() -> u64 {
return 5;
true
} {
_ => 42,
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = "core"
source = "path+from-root-DFCFC85C94C0C9BD"

[[package]]
name = "std"
source = "path+from-root-DFCFC85C94C0C9BD"
dependencies = ["core"]

[[package]]
name = "unify_never"
source = "member"
dependencies = ["std"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
license = "Apache-2.0"
name = "unify_never"
entry = "main.sw"

[dependencies]
std = { path = "../../../../reduced_std_libs/sway-lib-std-assert" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
script;

// Test of unification of the Never type

fn test_1(){
let mut v1: u32 = 1;
v1 = return;
}

fn test_2(){
let mut v2: u32 = return;
v2 = 1;
}


fn main() {
test_1();
test_2();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
category = "compile"
expected_warnings = 1
Loading