Skip to content

Commit 491a07f

Browse files
Fixes ReplaceDecls to no longer depend on filter hack. (#5541)
`ReplaceDecls` was using a hack to replace some function refs based on the self function parameter, this didn't work for associated functions. Now DeclMapping sources also contain a typeid that guarantees that the replacement is done to the correct declaration in case we have nested trait methods with the same name replaced. Fixes #5504 ## 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: Joshua Batty <joshpbatty@gmail.com>
1 parent 335754f commit 491a07f

File tree

18 files changed

+321
-88
lines changed

18 files changed

+321
-88
lines changed

sway-core/src/decl_engine/associated_item_decl_id.rs

+37-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
use sway_error::error::CompileError;
2-
use sway_types::Span;
2+
use sway_types::{Named, Span, Spanned};
33

44
use crate::{
55
decl_engine::*,
6+
engine_threading::DisplayWithEngines,
67
language::ty::{self, TyFunctionDecl},
8+
Engines,
79
};
810

911
#[derive(Debug, Eq, PartialEq, Hash, Clone)]
@@ -14,6 +16,17 @@ pub enum AssociatedItemDeclId {
1416
Type(DeclId<ty::TyTraitType>),
1517
}
1618

19+
impl AssociatedItemDeclId {
20+
pub fn span(&self, engines: &Engines) -> Span {
21+
match self {
22+
Self::TraitFn(decl_id) => engines.de().get(decl_id).span(),
23+
Self::Function(decl_id) => engines.de().get(decl_id).span(),
24+
Self::Constant(decl_id) => engines.de().get(decl_id).span(),
25+
Self::Type(decl_id) => engines.de().get(decl_id).span(),
26+
}
27+
}
28+
}
29+
1730
impl From<DeclId<ty::TyFunctionDecl>> for AssociatedItemDeclId {
1831
fn from(val: DeclId<ty::TyFunctionDecl>) -> Self {
1932
Self::Function(val)
@@ -97,6 +110,29 @@ impl std::fmt::Display for AssociatedItemDeclId {
97110
}
98111
}
99112

113+
impl DisplayWithEngines for AssociatedItemDeclId {
114+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>, engines: &Engines) -> std::fmt::Result {
115+
match self {
116+
Self::TraitFn(decl_id) => {
117+
write!(
118+
f,
119+
"decl(trait function {})",
120+
engines.de().get(decl_id).name()
121+
)
122+
}
123+
Self::Function(decl_id) => {
124+
write!(f, "decl(function {})", engines.de().get(decl_id).name())
125+
}
126+
Self::Constant(decl_id) => {
127+
write!(f, "decl(constant {})", engines.de().get(decl_id).name())
128+
}
129+
Self::Type(decl_id) => {
130+
write!(f, "decl(type {})", engines.de().get(decl_id).name())
131+
}
132+
}
133+
}
134+
}
135+
100136
impl TryFrom<DeclRefMixedFunctional> for DeclRefFunction {
101137
type Error = CompileError;
102138
fn try_from(value: DeclRefMixedFunctional) -> Result<Self, Self::Error> {

sway-core/src/decl_engine/mapping.rs

+69-47
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
1-
use std::fmt;
1+
use std::{collections::HashSet, fmt};
2+
3+
use sway_error::handler::{ErrorEmitted, Handler};
24

35
use crate::{
6+
engine_threading::DebugWithEngines,
47
language::ty::{TyTraitInterfaceItem, TyTraitItem},
58
Engines, TypeId, UnifyCheck,
69
};
710

8-
use super::{AssociatedItemDeclId, DeclEngineGet, InterfaceItemMap, ItemMap};
11+
use super::{AssociatedItemDeclId, InterfaceItemMap, ItemMap};
912

10-
type SourceDecl = AssociatedItemDeclId;
13+
type SourceDecl = (AssociatedItemDeclId, TypeId);
1114
type DestinationDecl = AssociatedItemDeclId;
1215

1316
/// The [DeclMapping] is used to create a mapping between a [SourceDecl] (LHS)
1417
/// and a [DestinationDecl] (RHS).
18+
#[derive(Clone)]
1519
pub struct DeclMapping {
1620
mapping: Vec<(SourceDecl, DestinationDecl)>,
1721
}
@@ -26,7 +30,7 @@ impl fmt::Display for DeclMapping {
2630
.map(|(source_type, dest_type)| {
2731
format!(
2832
"{} -> {}",
29-
source_type,
33+
source_type.0,
3034
match dest_type {
3135
AssociatedItemDeclId::TraitFn(decl_id) => decl_id.inner(),
3236
AssociatedItemDeclId::Function(decl_id) => decl_id.inner(),
@@ -55,17 +59,27 @@ impl fmt::Debug for DeclMapping {
5559
}
5660
}
5761

58-
impl DeclMapping {
59-
pub(crate) fn new() -> Self {
60-
Self {
61-
mapping: Vec::new(),
62-
}
63-
}
64-
65-
pub(crate) fn insert(&mut self, k: SourceDecl, v: DestinationDecl) {
66-
self.mapping.push((k, v))
62+
impl DebugWithEngines for DeclMapping {
63+
fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: &Engines) -> fmt::Result {
64+
write!(
65+
f,
66+
"DeclMapping {{ {} }}",
67+
self.mapping
68+
.iter()
69+
.map(|(source_type, dest_type)| {
70+
format!(
71+
"{} -> {}",
72+
engines.help_out(source_type.0.clone()),
73+
engines.help_out(dest_type)
74+
)
75+
})
76+
.collect::<Vec<_>>()
77+
.join(", ")
78+
)
6779
}
80+
}
6881

82+
impl DeclMapping {
6983
pub(crate) fn is_empty(&self) -> bool {
7084
self.mapping.is_empty()
7185
}
@@ -79,9 +93,15 @@ impl DeclMapping {
7993
for (interface_decl_name, interface_item) in interface_decl_refs.into_iter() {
8094
if let Some(new_item) = impld_decl_refs.get(&interface_decl_name) {
8195
let interface_decl_ref = match interface_item {
82-
TyTraitInterfaceItem::TraitFn(decl_ref) => decl_ref.id().into(),
83-
TyTraitInterfaceItem::Constant(decl_ref) => decl_ref.id().into(),
84-
TyTraitInterfaceItem::Type(decl_ref) => decl_ref.id().into(),
96+
TyTraitInterfaceItem::TraitFn(decl_ref) => {
97+
(decl_ref.id().into(), interface_decl_name.1)
98+
}
99+
TyTraitInterfaceItem::Constant(decl_ref) => {
100+
(decl_ref.id().into(), interface_decl_name.1)
101+
}
102+
TyTraitInterfaceItem::Type(decl_ref) => {
103+
(decl_ref.id().into(), interface_decl_name.1)
104+
}
85105
};
86106
let new_decl_ref = match new_item {
87107
TyTraitItem::Fn(decl_ref) => decl_ref.id().into(),
@@ -94,9 +114,9 @@ impl DeclMapping {
94114
for (decl_name, item) in item_decl_refs.into_iter() {
95115
if let Some(new_item) = impld_decl_refs.get(&decl_name) {
96116
let interface_decl_ref = match item {
97-
TyTraitItem::Fn(decl_ref) => decl_ref.id().into(),
98-
TyTraitItem::Constant(decl_ref) => decl_ref.id().into(),
99-
TyTraitItem::Type(decl_ref) => decl_ref.id().into(),
117+
TyTraitItem::Fn(decl_ref) => (decl_ref.id().into(), decl_name.1),
118+
TyTraitItem::Constant(decl_ref) => (decl_ref.id().into(), decl_name.1),
119+
TyTraitItem::Type(decl_ref) => (decl_ref.id().into(), decl_name.1),
100120
};
101121
let new_decl_ref = match new_item {
102122
TyTraitItem::Fn(decl_ref) => decl_ref.id().into(),
@@ -109,39 +129,41 @@ impl DeclMapping {
109129
DeclMapping { mapping }
110130
}
111131

112-
pub(crate) fn find_match(&self, decl_ref: SourceDecl) -> Option<DestinationDecl> {
113-
for (source_decl_ref, dest_decl_ref) in self.mapping.iter() {
114-
if *source_decl_ref == decl_ref {
115-
return Some(dest_decl_ref.clone());
116-
}
117-
}
118-
None
119-
}
120-
121-
/// This method returns only associated item functions that have as self type the given type.
122-
pub(crate) fn filter_functions_by_self_type(
132+
pub(crate) fn find_match(
123133
&self,
124-
self_type: TypeId,
134+
_handler: &Handler,
125135
engines: &Engines,
126-
) -> DeclMapping {
127-
let mut mapping: Vec<(SourceDecl, DestinationDecl)> = vec![];
128-
for (source_decl_ref, dest_decl_ref) in self.mapping.iter().cloned() {
129-
match dest_decl_ref {
130-
AssociatedItemDeclId::TraitFn(_) => mapping.push((source_decl_ref, dest_decl_ref)),
131-
AssociatedItemDeclId::Function(func_id) => {
132-
let func = engines.de().get(&func_id);
136+
decl_ref: AssociatedItemDeclId,
137+
typeid: Option<TypeId>,
138+
self_typeid: Option<TypeId>,
139+
) -> Result<Option<DestinationDecl>, ErrorEmitted> {
140+
let mut dest_decl_refs = HashSet::<DestinationDecl>::new();
133141

134-
let unify_check = UnifyCheck::non_dynamic_equality(engines);
135-
if let (left, Some(right)) = (self_type, func.parameters.first()) {
136-
if unify_check.check(left, right.type_argument.type_id) {
137-
mapping.push((source_decl_ref, dest_decl_ref));
138-
}
139-
}
142+
if let Some(mut typeid) = typeid {
143+
if engines.te().get(typeid).is_self_type() && self_typeid.is_some() {
144+
// If typeid is `Self`, then we use the self_typeid instead.
145+
typeid = self_typeid.unwrap();
146+
}
147+
for (source_decl_ref, dest_decl_ref) in self.mapping.iter() {
148+
let unify_check = UnifyCheck::non_dynamic_equality(engines);
149+
if source_decl_ref.0 == decl_ref && unify_check.check(source_decl_ref.1, typeid) {
150+
dest_decl_refs.insert(dest_decl_ref.clone());
140151
}
141-
AssociatedItemDeclId::Constant(_) => mapping.push((source_decl_ref, dest_decl_ref)),
142-
AssociatedItemDeclId::Type(_) => mapping.push((source_decl_ref, dest_decl_ref)),
143152
}
144153
}
145-
DeclMapping { mapping }
154+
155+
// At most one replacement should be found for decl_ref.
156+
/* TODO uncomment this and close issue #5540
157+
if dest_decl_refs.len() > 1 {
158+
handler.emit_err(CompileError::InternalOwned(
159+
format!(
160+
"Multiple replacements for decl {} implemented in {}",
161+
engines.help_out(decl_ref),
162+
engines.help_out(typeid),
163+
),
164+
dest_decl_refs.iter().last().unwrap().span(engines),
165+
));
166+
}*/
167+
Ok(dest_decl_refs.iter().next().cloned())
146168
}
147169
}

sway-core/src/decl_engine/ref.rs

+18-3
Original file line numberDiff line numberDiff line change
@@ -294,20 +294,35 @@ impl ReplaceDecls for DeclRefFunction {
294294
fn replace_decls_inner(
295295
&mut self,
296296
decl_mapping: &DeclMapping,
297-
_handler: &Handler,
297+
handler: &Handler,
298298
ctx: &mut TypeCheckContext,
299299
) -> Result<(), ErrorEmitted> {
300300
let engines = ctx.engines();
301301
let decl_engine = engines.de();
302-
if let Some(new_decl_ref) = decl_mapping.find_match(self.id.into()) {
302+
303+
let func = decl_engine.get(self);
304+
305+
if let Some(new_decl_ref) = decl_mapping.find_match(
306+
handler,
307+
ctx.engines(),
308+
self.id.into(),
309+
func.implementing_for_typeid,
310+
ctx.self_type(),
311+
)? {
303312
if let AssociatedItemDeclId::Function(new_decl_ref) = new_decl_ref {
304313
self.id = new_decl_ref;
305314
}
306315
return Ok(());
307316
}
308317
let all_parents = decl_engine.find_all_parents(engines, &self.id);
309318
for parent in all_parents.iter() {
310-
if let Some(new_decl_ref) = decl_mapping.find_match(parent.clone()) {
319+
if let Some(new_decl_ref) = decl_mapping.find_match(
320+
handler,
321+
ctx.engines(),
322+
parent.clone(),
323+
func.implementing_for_typeid,
324+
ctx.self_type(),
325+
)? {
311326
if let AssociatedItemDeclId::Function(new_decl_ref) = new_decl_ref {
312327
self.id = new_decl_ref;
313328
}

sway-core/src/language/ty/declaration/function.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub struct TyFunctionDecl {
3030
pub body: TyCodeBlock,
3131
pub parameters: Vec<TyFunctionParameter>,
3232
pub implementing_type: Option<TyDecl>,
33+
pub implementing_for_typeid: Option<TypeId>,
3334
pub span: Span,
3435
pub call_path: CallPath,
3536
pub attributes: transform::AttributesMap,
@@ -159,6 +160,7 @@ impl HashWithEngines for TyFunctionDecl {
159160
span: _,
160161
attributes: _,
161162
implementing_type: _,
163+
implementing_for_typeid: _,
162164
where_clause: _,
163165
is_trait_method_dummy: _,
164166
} = self;
@@ -183,6 +185,9 @@ impl SubstTypes for TyFunctionDecl {
183185
.for_each(|x| x.subst(type_mapping, engines));
184186
self.return_type.subst(type_mapping, engines);
185187
self.body.subst(type_mapping, engines);
188+
if let Some(implementing_for) = self.implementing_for_typeid.as_mut() {
189+
implementing_for.subst(type_mapping, engines);
190+
}
186191
}
187192
}
188193

@@ -193,7 +198,9 @@ impl ReplaceDecls for TyFunctionDecl {
193198
handler: &Handler,
194199
ctx: &mut TypeCheckContext,
195200
) -> Result<(), ErrorEmitted> {
196-
self.body.replace_decls(decl_mapping, handler, ctx)
201+
let mut func_ctx = ctx.by_ref().with_self_type(self.implementing_for_typeid);
202+
self.body
203+
.replace_decls(decl_mapping, handler, &mut func_ctx)
197204
}
198205
}
199206

@@ -296,6 +303,7 @@ impl TyFunctionDecl {
296303
name: name.clone(),
297304
body: TyCodeBlock::default(),
298305
implementing_type: None,
306+
implementing_for_typeid: None,
299307
span: span.clone(),
300308
call_path: CallPath::from(Ident::dummy()),
301309
attributes: Default::default(),

sway-core/src/language/ty/declaration/trait.rs

+31-5
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use sway_types::{Ident, Named, Span, Spanned};
88

99
use crate::{
1010
decl_engine::{
11-
mapping::DeclMapping, DeclEngineReplace, DeclRefConstant, DeclRefFunction, DeclRefTraitFn,
12-
DeclRefTraitType, ReplaceFunctionImplementingType,
11+
DeclEngineReplace, DeclRefConstant, DeclRefFunction, DeclRefTraitFn, DeclRefTraitType,
12+
ReplaceFunctionImplementingType,
1313
},
1414
engine_threading::*,
1515
language::{parsed, CallPath, Visibility},
@@ -44,6 +44,35 @@ pub enum TyTraitInterfaceItem {
4444
Type(DeclRefTraitType),
4545
}
4646

47+
impl DisplayWithEngines for TyTraitInterfaceItem {
48+
fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: &Engines) -> fmt::Result {
49+
write!(f, "{:?}", engines.help_out(self))
50+
}
51+
}
52+
53+
impl DebugWithEngines for TyTraitInterfaceItem {
54+
fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: &Engines) -> fmt::Result {
55+
write!(
56+
f,
57+
"TyTraitItem {}",
58+
match self {
59+
TyTraitInterfaceItem::TraitFn(fn_ref) => format!(
60+
"fn {:?}",
61+
engines.help_out(&*engines.de().get_trait_fn(fn_ref))
62+
),
63+
TyTraitInterfaceItem::Constant(const_ref) => format!(
64+
"const {:?}",
65+
engines.help_out(&*engines.de().get_constant(const_ref))
66+
),
67+
TyTraitInterfaceItem::Type(type_ref) => format!(
68+
"type {:?}",
69+
engines.help_out(&*engines.de().get_type(type_ref))
70+
),
71+
}
72+
)
73+
}
74+
}
75+
4776
#[derive(Clone, Debug)]
4877
pub enum TyTraitItem {
4978
Fn(DeclRefFunction),
@@ -242,7 +271,6 @@ impl Spanned for TyTraitItem {
242271

243272
impl SubstTypes for TyTraitDecl {
244273
fn subst_inner(&mut self, type_mapping: &TypeSubstMap, engines: &Engines) {
245-
let mut decl_mapping = DeclMapping::new();
246274
self.type_parameters
247275
.iter_mut()
248276
.for_each(|x| x.subst(type_mapping, engines));
@@ -253,14 +281,12 @@ impl SubstTypes for TyTraitDecl {
253281
let new_item_ref = item_ref
254282
.clone()
255283
.subst_types_and_insert_new_with_parent(type_mapping, engines);
256-
decl_mapping.insert(item_ref.id().into(), new_item_ref.id().into());
257284
item_ref.replace_id(*new_item_ref.id());
258285
}
259286
TyTraitInterfaceItem::Constant(decl_ref) => {
260287
let new_decl_ref = decl_ref
261288
.clone()
262289
.subst_types_and_insert_new(type_mapping, engines);
263-
decl_mapping.insert(decl_ref.id().into(), new_decl_ref.id().into());
264290
decl_ref.replace_id(*new_decl_ref.id());
265291
}
266292
TyTraitInterfaceItem::Type(decl_ref) => {

0 commit comments

Comments
 (0)