Skip to content

Commit

Permalink
Fix handling of associated constants in ABI/traits (#6768)
Browse files Browse the repository at this point in the history 
## Description

[Fix handling of initialized associated constants in ABIs and
traits.](88fee13)

[Fix impl trait associated constant reference wrongly resolving to the
ABI
value](3a401f6)

Fixes #6343 and
#6310.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [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: Sophie Dankel <[email protected]>
Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
@tritao @sdankel @JoshuaBatty
3 people authored Dec 4, 2024
1 parent 48f633d commit 0ccf47b
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 79 deletions.
47 changes: 28 additions & 19 deletions sway-core/src/semantic_analysis/ast_node/declaration/abi.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};

use sway_error::error::CompileError;
use sway_types::{Ident, Named, Span, Spanned};
Expand Down Expand Up @@ -163,31 +163,19 @@ impl ty::TyAbiDecl {
ctx.engines.de().insert(const_decl.clone(), Some(&decl_id));
new_interface_surface
.push(ty::TyTraitInterfaceItem::Constant(decl_ref.clone()));

let const_name = const_decl.call_path.suffix.clone();
ctx.insert_symbol(
handler,
const_name.clone(),
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
decl_id: *decl_ref.id(),
}),
)?;

const_name
const_decl.call_path.suffix.clone()
}
TraitItem::Type(decl_id) => {
let type_decl = engines.pe().get_trait_type(&decl_id).as_ref().clone();
handler.emit_err(CompileError::AssociatedTypeNotSupportedInAbi {
span: type_decl.span.clone(),
});

let type_decl =
ty::TyTraitType::type_check(handler, ctx.by_ref(), type_decl)?;
let decl_ref =
ctx.engines().de().insert(type_decl.clone(), Some(&decl_id));
new_interface_surface
.push(ty::TyTraitInterfaceItem::Type(decl_ref.clone()));

type_decl.name
}
TraitItem::Error(_, _) => {
Expand Down Expand Up @@ -276,6 +264,8 @@ impl ty::TyAbiDecl {
(false, Span::dummy())
};

let mut const_symbols = HashMap::<Ident, ty::TyDecl>::new();

handler.scope(|handler| {
for item in interface_surface.iter() {
match item {
Expand Down Expand Up @@ -339,9 +329,8 @@ impl ty::TyAbiDecl {
let const_decl = decl_engine.get_constant(decl_ref);
let const_name = const_decl.call_path.suffix.clone();
all_items.push(TyImplItem::Constant(decl_ref.clone()));
let _ = ctx.insert_symbol(
handler,
const_name.clone(),
const_symbols.insert(
const_name,
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
decl_id: *decl_ref.id(),
}),
Expand Down Expand Up @@ -397,10 +386,24 @@ impl ty::TyAbiDecl {
}
ty::TyTraitItem::Constant(decl_ref) => {
let const_decl = decl_engine.get_constant(decl_ref);
all_items.push(TyImplItem::Constant(decl_engine.insert_arc(
let const_name = const_decl.name().clone();
let const_has_value = const_decl.value.is_some();
let decl_id = decl_engine.insert_arc(
const_decl,
decl_engine.get_parsed_decl_id(decl_ref.id()).as_ref(),
)));
);
all_items.push(TyImplItem::Constant(decl_id.clone()));

// If this non-interface item has a value, then we want to overwrite the
// the previously inserted constant symbol from the interface surface.
if const_has_value {
const_symbols.insert(
const_name,
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
decl_id: *decl_id.id(),
}),
);
}
}
ty::TyTraitItem::Type(decl_ref) => {
let type_decl = decl_engine.get_type(decl_ref);
Expand All @@ -411,6 +414,12 @@ impl ty::TyAbiDecl {
}
}
}

// Insert the constants into the namespace.
for (name, decl) in const_symbols.into_iter() {
let _ = ctx.insert_symbol(handler, name, decl);
}

// Insert the methods of the ABI into the namespace.
// Specifically do not check for conflicting definitions because
// this is just a temporary namespace for type checking and
Expand Down
143 changes: 112 additions & 31 deletions sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@ use sway_error::{
error::{CompileError, InterfaceName},
handler::{ErrorEmitted, Handler},
};
use sway_types::{Ident, Span, Spanned};
use sway_types::{Ident, Named, Span, Spanned};

use crate::{
decl_engine::{parsed_id::ParsedDeclId, *},
engine_threading::*,
language::{
parsed::*,
ty::{
self, TyConstantDecl, TyDecl, TyFunctionDecl, TyImplItem, TyImplSelfOrTrait,
TyTraitInterfaceItem, TyTraitItem, TyTraitType,
self, ConstantDecl, TyConstantDecl, TyDecl, TyFunctionDecl, TyImplItem,
TyImplSelfOrTrait, TyTraitInterfaceItem, TyTraitItem, TyTraitType,
},
*,
},
Expand Down Expand Up @@ -888,6 +888,54 @@ fn type_check_trait_implementation(
}
}

for item in impl_items {
match item {
ImplItem::Fn(_impl_method_id) => {}
ImplItem::Constant(decl_id) => {
let const_decl = engines.pe().get_constant(decl_id).as_ref().clone();
let mut const_decl = type_check_const_decl(
handler,
ctx.by_ref().with_type_subst(&trait_type_mapping),
&const_decl,
trait_name,
is_contract,
&impld_item_refs,
&constant_checklist,
)
.unwrap_or_else(|_| ty::TyConstantDecl::error(ctx.engines(), const_decl.clone()));

const_decl.subst(&SubstTypesContext::new(
engines,
&trait_type_mapping,
!ctx.code_block_first_pass(),
));

// Remove this constant from the checklist.
let name = const_decl.call_path.suffix.clone();
constant_checklist.remove(&name);

// Add this constant to the "impld decls".
let decl_ref = decl_engine.insert(const_decl, Some(decl_id));
impld_item_refs.insert(
(name.clone(), implementing_for),
TyTraitItem::Constant(decl_ref.clone()),
);

let prev_const_shadowing_mode = ctx.const_shadowing_mode;
ctx.const_shadowing_mode = ConstShadowingMode::Allow;
let _ = ctx.insert_symbol(
handler,
name,
TyDecl::ConstantDecl(ConstantDecl {
decl_id: *decl_ref.id(),
}),
);
ctx.const_shadowing_mode = prev_const_shadowing_mode;
}
ImplItem::Type(_) => {}
}
}

for item in impl_items {
match item {
ImplItem::Fn(impl_method_id) => {
Expand Down Expand Up @@ -919,33 +967,7 @@ fn type_check_trait_implementation(
let decl_ref = decl_engine.insert(impl_method, Some(impl_method_id));
impld_item_refs.insert((name, implementing_for), TyTraitItem::Fn(decl_ref));
}
ImplItem::Constant(decl_id) => {
let const_decl = engines.pe().get_constant(decl_id).as_ref().clone();
let mut const_decl = type_check_const_decl(
handler,
ctx.by_ref().with_type_subst(&trait_type_mapping),
&const_decl,
trait_name,
is_contract,
&impld_item_refs,
&constant_checklist,
)
.unwrap_or_else(|_| ty::TyConstantDecl::error(ctx.engines(), const_decl.clone()));

const_decl.subst(&SubstTypesContext::new(
engines,
&trait_type_mapping,
!ctx.code_block_first_pass(),
));

// Remove this constant from the checklist.
let name = const_decl.call_path.suffix.clone();
constant_checklist.remove(&name);

// Add this constant to the "impld decls".
let decl_ref = decl_engine.insert(const_decl, Some(decl_id));
impld_item_refs.insert((name, implementing_for), TyTraitItem::Constant(decl_ref));
}
ImplItem::Constant(_decl_id) => {}
ImplItem::Type(_) => {}
}
}
Expand Down Expand Up @@ -1305,6 +1327,50 @@ fn type_check_impl_method(
})
}

fn trait_const_item_value(
trait_decl: TyDecl,
engines: &Engines,
const_decl: &TyConstantDecl,
) -> Option<ty::TyExpression> {
fn get_const_decl_from_trait_items(
engines: &Engines,
items: &[TyTraitInterfaceItem],
name: &Ident,
) -> Option<Arc<TyConstantDecl>> {
for item in items.iter() {
match item {
TyTraitInterfaceItem::Constant(decl) => {
let const_decl = engines.de().get_constant(decl.id());
if const_decl.name() == name {
return Some(const_decl);
}
}
_ => continue,
}
}
None
}

let trait_or_abi_const_decl = match trait_decl {
TyDecl::TraitDecl(decl) => get_const_decl_from_trait_items(
engines,
&engines.de().get_trait(&decl.decl_id).interface_surface,
const_decl.name(),
),
TyDecl::AbiDecl(decl) => get_const_decl_from_trait_items(
engines,
&engines.de().get_abi(&decl.decl_id).interface_surface,
const_decl.name(),
),
_ => unreachable!(),
};

match trait_or_abi_const_decl {
Some(trait_or_abi_const_decl) => trait_or_abi_const_decl.value.clone(),
None => None,
}
}

#[allow(clippy::too_many_arguments)]
fn type_check_const_decl(
handler: &Handler,
Expand Down Expand Up @@ -1336,10 +1402,25 @@ fn type_check_const_decl(
};

// type check the constant declaration
let const_decl = ty::TyConstantDecl::type_check(handler, ctx.by_ref(), const_decl.clone())?;
let mut const_decl = ty::TyConstantDecl::type_check(handler, ctx.by_ref(), const_decl.clone())?;

let const_name = const_decl.call_path.suffix.clone();

// Ensure that there is an expression if the constant in the base trait is not defined.
let trait_decl = ctx.resolve_call_path(handler, trait_name)?;

let trait_const_item_value = trait_const_item_value(trait_decl, engines, &const_decl);
if trait_const_item_value.is_none() && const_decl.value.is_none() {
return Err(handler.emit_err(CompileError::ConstantRequiresExpression {
span: const_decl.span.clone(),
}));
}

// Ensure the constant decl has a value, if none was set then it inherits the base trait/abi value.
if const_decl.value.is_none() {
const_decl.value = trait_const_item_value;
}

// Ensure that there aren't multiple definitions of this constant
if impld_constant_ids.contains_key(&(const_name.clone(), self_type_id)) {
return Err(
Expand Down
36 changes: 28 additions & 8 deletions sway-core/src/semantic_analysis/ast_node/declaration/trait.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use std::collections::{BTreeMap, HashSet};
use std::collections::{BTreeMap, HashMap, HashSet};

use parsed_id::ParsedDeclId;
use sway_error::{
error::CompileError,
handler::{ErrorEmitted, Handler},
warning::{CompileWarning, Warning},
};
use sway_types::{style::is_upper_camel_case, Ident, Spanned};
use sway_types::{style::is_upper_camel_case, Ident, Named, Spanned};

use crate::{
decl_engine::*,
Expand Down Expand Up @@ -510,6 +510,8 @@ impl TyTraitDecl {
.collect(),
);

let mut const_symbols = HashMap::<Ident, ty::TyDecl>::new();

for item in interface_surface.iter() {
match item {
ty::TyTraitInterfaceItem::TraitFn(decl_ref) => {
Expand All @@ -527,11 +529,10 @@ impl TyTraitDecl {
}
ty::TyTraitInterfaceItem::Constant(decl_ref) => {
let const_decl = decl_engine.get_constant(decl_ref);
let const_name = const_decl.call_path.suffix.clone();
all_items.push(TyImplItem::Constant(decl_ref.clone()));
let _ = ctx.insert_symbol(
handler,
const_name.clone(),
let const_name = const_decl.call_path.suffix.clone();
const_symbols.insert(
const_name,
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
decl_id: *decl_ref.id(),
}),
Expand Down Expand Up @@ -568,10 +569,24 @@ impl TyTraitDecl {
&type_mapping,
!ctx.code_block_first_pass(),
));
all_items.push(TyImplItem::Constant(decl_engine.insert(
let const_name = const_decl.name().clone();
let const_has_value = const_decl.value.is_some();
let decl_id = decl_engine.insert(
const_decl,
decl_engine.get_parsed_decl_id(decl_ref.id()).as_ref(),
)));
);
all_items.push(TyImplItem::Constant(decl_id.clone()));

// If this non-interface item has a value, then we want to overwrite the
// the previously inserted constant symbol from the interface surface.
if const_has_value {
const_symbols.insert(
const_name,
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
decl_id: *decl_id.id(),
}),
);
}
}
ty::TyTraitItem::Type(decl_ref) => {
let mut type_decl = (*decl_engine.get_type(decl_ref)).clone();
Expand All @@ -588,6 +603,11 @@ impl TyTraitDecl {
}
}

// Insert the constants into the namespace.
for (name, decl) in const_symbols.into_iter() {
let _ = ctx.insert_symbol(handler, name, decl);
}

// Insert the methods of the trait into the namespace.
// Specifically do not check for conflicting definitions because
// this is just a temporary namespace for type checking and
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/ast_node/modes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub enum AbiMode {

#[derive(Clone, Copy, PartialEq, Eq, Default)]
pub enum ConstShadowingMode {
Allow,
Sequential,
#[default]
ItemStyle,
Expand Down
3 changes: 3 additions & 0 deletions sway-core/src/semantic_analysis/namespace/lexical_scope.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,9 @@ impl Items {
is_alias: bool,
item: &ResolvedDeclaration,
const_shadowing_mode: ConstShadowingMode| {
if const_shadowing_mode == ConstShadowingMode::Allow {
return;
}
match (decl, item) {
// TODO: Do not handle any shadowing errors while handling parsed declarations yet,
// or else we will emit errors in a different order from the source code order.
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/type_check_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub struct TypeCheckContext<'a> {
/// Whether or not a const declaration shadows previous const declarations sequentially.
///
/// This is `Sequential` while checking const declarations in functions, otherwise `ItemStyle`.
const_shadowing_mode: ConstShadowingMode,
pub(crate) const_shadowing_mode: ConstShadowingMode,
/// Whether or not a generic type parameters shadows previous generic type parameters.
///
/// This is `Disallow` everywhere except while checking type parameters bounds in struct instantiation.
Expand Down
Loading

0 comments on commit 0ccf47b

Please sign in to comment.