Skip to content

Commit

Permalink
Fix CopyTypes implementation for DeclarationId. (FuelLabs#2874)
Browse files Browse the repository at this point in the history
This fixes a regression in CopyTypes which was preventing the mutated
declarations from reaching the `DeclarationEngine`.

After calling `copy_types` on the declaration we now replace it inside
the declaration engine.

Notes: this was found as part of debugging a test failure for
FuelLabs#2786. This fix makes the test
pass the type checking phase which it was failing, and now fails in IR
gen, so there's probably still issues around this or another related
area.
  • Loading branch information
tritao authored Sep 27, 2022
1 parent 44f6bcb commit c5e5433
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
18 changes: 17 additions & 1 deletion sway-core/src/concurrent_slab.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use std::sync::RwLock;

use crate::{type_system::TypeId, TypeInfo};
use crate::{
declaration_engine::{declaration_id::DeclarationId, declaration_wrapper::DeclarationWrapper},
type_system::TypeId,
TypeInfo,
};

#[derive(Debug)]
pub(crate) struct ConcurrentSlab<T> {
Expand Down Expand Up @@ -75,3 +79,15 @@ impl ConcurrentSlab<TypeInfo> {
None
}
}

impl ConcurrentSlab<DeclarationWrapper> {
pub fn replace(
&self,
index: DeclarationId,
new_value: DeclarationWrapper,
) -> Option<DeclarationWrapper> {
let mut inner = self.inner.write().unwrap();
inner[*index] = new_value;
None
}
}
8 changes: 8 additions & 0 deletions sway-core/src/declaration_engine/declaration_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ impl DeclarationEngine {
self.slab.get(*index)
}

fn replace_decl_id(&self, index: DeclarationId, wrapper: DeclarationWrapper) {
self.slab.replace(index, wrapper);
}

fn add_monomorphized_copy(&self, original_id: DeclarationId, new_id: DeclarationId) {
let mut monomorphized_copies = self.monomorphized_copies.write().unwrap();
match monomorphized_copies.get_mut(&*original_id) {
Expand Down Expand Up @@ -255,6 +259,10 @@ pub(crate) fn de_look_up_decl_id(index: DeclarationId) -> DeclarationWrapper {
DECLARATION_ENGINE.look_up_decl_id(index)
}

pub(crate) fn de_replace_decl_id(index: DeclarationId, wrapper: DeclarationWrapper) {
DECLARATION_ENGINE.replace_decl_id(index, wrapper)
}

pub(crate) fn de_add_monomorphized_copy(original_id: DeclarationId, new_id: DeclarationId) {
DECLARATION_ENGINE.add_monomorphized_copy(original_id, new_id);
}
Expand Down
6 changes: 4 additions & 2 deletions sway-core/src/declaration_engine/declaration_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use sway_types::{Span, Spanned};

use crate::type_system::{CopyTypes, TypeMapping};

use super::declaration_engine::de_look_up_decl_id;
use super::declaration_engine::{de_look_up_decl_id, de_replace_decl_id};

/// An ID used to refer to an item in the [DeclarationEngine](super::declaration_engine::DeclarationEngine)
#[derive(Debug, Eq)]
Expand Down Expand Up @@ -53,7 +53,9 @@ impl Spanned for DeclarationId {

impl CopyTypes for DeclarationId {
fn copy_types(&mut self, type_mapping: &TypeMapping) {
de_look_up_decl_id(self.clone()).copy_types(type_mapping)
let mut decl = de_look_up_decl_id(self.clone());
decl.copy_types(type_mapping);
de_replace_decl_id(self.clone(), decl);
}
}

Expand Down

0 comments on commit c5e5433

Please sign in to comment.