Skip to content

Commit

Permalink
Fixes regression caused by enum and struct callpaths. (FuelLabs#4007)
Browse files Browse the repository at this point in the history
## Description

The issue is that struct or enum can be imported with different
callpaths depending the module they are imported from.

For instance we could have a struct with callpath
`my_script::data_structures::SomeStruct` in a module and in another
module as `my_script::eq_impls::data_structures::SomeStruct`. This makes
callpaths unreliable to compare declarations.

Closes FuelLabs#3998

Reopens FuelLabs#418

Disables tests multiple_enums_with_the_same_name and
multiple_structs_with_the_same_name because they relied in callpath
comparisons to work.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Co-authored-by: Mohammad Fawaz <[email protected]>
  • Loading branch information
esdrubal and mohammadfawaz authored Feb 7, 2023
1 parent 2a8837d commit 7e861e4
Show file tree
Hide file tree
Showing 11 changed files with 166 additions and 16 deletions.
32 changes: 20 additions & 12 deletions sway-core/src/type_system/unify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,9 @@ use sway_error::{
type_error::TypeError,
warning::{CompileWarning, Warning},
};
use sway_types::{integer_bits::IntegerBits, Span, Spanned};
use sway_types::{integer_bits::IntegerBits, Ident, Span, Spanned};

use crate::{
engine_threading::*,
language::{ty, CallPath},
type_system::*,
};
use crate::{engine_threading::*, language::ty, type_system::*};

/// Helper struct to aid in type unification.
pub(super) struct Unifier<'a> {
Expand Down Expand Up @@ -120,7 +116,13 @@ impl<'a> Unifier<'a> {
type_parameters: etps,
fields: efs,
},
) => self.unify_structs(received, expected, span, (rn, rpts, rfs), (en, etps, efs)),
) => self.unify_structs(
received,
expected,
span,
(rn.suffix, rpts, rfs),
(en.suffix, etps, efs),
),
// Let empty enums to coerce to any other type. This is useful for Never enum.
(
Enum {
Expand All @@ -139,7 +141,13 @@ impl<'a> Unifier<'a> {
type_parameters: etps,
variant_types: evs,
},
) => self.unify_enums(received, expected, span, (rn, rtps, rvs), (en, etps, evs)),
) => self.unify_enums(
received,
expected,
span,
(rn.suffix, rtps, rvs),
(en.suffix, etps, evs),
),

// For integers and numerics, we (potentially) unify the numeric
// with the integer.
Expand Down Expand Up @@ -339,8 +347,8 @@ impl<'a> Unifier<'a> {
received: TypeId,
expected: TypeId,
span: &Span,
r: (CallPath, Vec<TypeParameter>, Vec<ty::TyStructField>),
e: (CallPath, Vec<TypeParameter>, Vec<ty::TyStructField>),
r: (Ident, Vec<TypeParameter>, Vec<ty::TyStructField>),
e: (Ident, Vec<TypeParameter>, Vec<ty::TyStructField>),
) -> (Vec<CompileWarning>, Vec<TypeError>) {
let mut warnings = vec![];
let mut errors = vec![];
Expand Down Expand Up @@ -388,8 +396,8 @@ impl<'a> Unifier<'a> {
received: TypeId,
expected: TypeId,
span: &Span,
r: (CallPath, Vec<TypeParameter>, Vec<ty::TyEnumVariant>),
e: (CallPath, Vec<TypeParameter>, Vec<ty::TyEnumVariant>),
r: (Ident, Vec<TypeParameter>, Vec<ty::TyEnumVariant>),
e: (Ident, Vec<TypeParameter>, Vec<ty::TyEnumVariant>),
) -> (Vec<CompileWarning>, Vec<TypeError>) {
let mut warnings = vec![];
let mut errors = vec![];
Expand Down
8 changes: 6 additions & 2 deletions sway-core/src/type_system/unify_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ impl<'a> UnifyCheck<'a> {
.iter()
.map(|x| x.type_id)
.collect::<Vec<_>>();
l_name == r_name && l_names == r_names && self.check_multiple(&l_types, &r_types)
l_name.suffix == r_name.suffix
&& l_names == r_names
&& self.check_multiple(&l_types, &r_types)
}
(
Struct {
Expand All @@ -228,7 +230,9 @@ impl<'a> UnifyCheck<'a> {
.iter()
.map(|x| x.type_id)
.collect::<Vec<_>>();
l_name == r_name && l_names == r_names && self.check_multiple(&l_types, &r_types)
l_name.suffix == r_name.suffix
&& l_names == r_names
&& self.check_multiple(&l_types, &r_types)
}

// For contract callers, they can be coerced if they have the same
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "fail"
category = "disabled"

# check: $()x = y;
# nextln: $()Mismatched types.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "fail"
category = "disabled"

# check: $()x = y;
# nextln: $()Mismatched types.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-9E94934D4E529F7D'

[[package]]
name = 'import_with_different_callpaths'
source = 'member'
dependencies = ['std']

[[package]]
name = 'std'
source = 'path+from-root-9E94934D4E529F7D'
dependencies = ['core']
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 = "import_with_different_callpaths"
entry = "main.sw"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"configurables": [],
"functions": [
{
"attributes": null,
"inputs": [],
"name": "main",
"output": {
"name": "",
"type": 0,
"typeArguments": null
}
}
],
"loggedTypes": [],
"messagesTypes": [],
"types": [
{
"components": [],
"type": "()",
"typeId": 0,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
library data_structures;

pub struct SomeStruct<T> {
a: T,
}

pub enum SomeEnum<T> {
a: T,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
library eq_impls;

dep data_structures;

use data_structures::{SomeEnum, SomeStruct};
use core::ops::Eq;

impl Eq for SomeEnum<u32> {
fn eq(self, other: Self) -> bool {
match self {
SomeEnum::a(val) => {
match other {
SomeEnum::a(other_val) => {
val == other_val
}
}
}
}
}
}

impl Eq for SomeStruct<u32> {
fn eq(self, other: Self) -> bool {
self.a == other.a
}
}

impl Eq for Vec<SomeStruct<u32>> {
fn eq(self, other: Self) -> bool {
if self.len() != other.len() {
return false;
}
let mut i = 0;
while i < self.len() {
if self.get(i).unwrap() != other.get(i).unwrap() {
return false;
}
i += 1;
}
true
}
}

impl Eq for Vec<SomeEnum<u32>> {
fn eq(self, other: Self) -> bool {
if self.len() != other.len() {
return false;
}

let mut i = 0;
while i < self.len() {
if self.get(i).unwrap() != other.get(i).unwrap() {
return false;
}
i += 1;
}
true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
script;

dep eq_impls;
dep data_structures;

use eq_impls::*;
use data_structures::*;

fn main() {
let mut expected = Vec::new();
expected.push(SomeEnum::a(0u32));
expected.push(SomeEnum::a(1u32));

assert(expected == expected);

let mut expected = Vec::new();
expected.push(SomeStruct { a: 0u32 });
expected.push(SomeStruct { a: 1u32 });

assert(expected == expected);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "run"
expected_result = { action = "return", value = 0 }
validate_abi = true

0 comments on commit 7e861e4

Please sign in to comment.