Skip to content

Commit

Permalink
Improved trait coherence checking.
Browse files Browse the repository at this point in the history
This commit refines trait coherence checking to guarantee that there is
only one valid implementation for each trait. It introduces improved
orphan rules and more rigorous overlap checks to prevent conflicting
trait implementations. With these changes, the system now more reliably
enforces coherence, ensuring that traits are implemented uniquely, and
reducing potential ambiguities.

This approach mirrors Rust’s design by enforcing two distinct safety and
coherence checks on trait implementations:

1. Orphan Rules Check
The orphan rules require that for any trait implementation, either the
trait or the type must be defined within the current package. This
restriction prevents external packages from implementing foreign traits
for foreign types, which could otherwise lead to conflicting
implementations and ambiguities across different parts of a codebase.
Essentially, it helps maintain clear ownership and boundaries of trait
implementations.

2. Overlap Impl Check
The overlap impl check ensures that no two trait implementations can
apply to the same type in an ambiguous manner. If two implementations
could potentially match the same type, it would be unclear which one
should be used, leading to coherence issues. By enforcing that
implementations do not overlap, the system guarantees that trait
resolution is unambiguous and predictable.

Together, these checks promote a robust and maintainable system by
ensuring that trait implementations are both locally controlled (orphan
rules) and non-conflicting (overlap check).

The test suite has been updated to comply with the new rules. However,
there is one current limitation regarding arrays. For arrays, the
coherence checks have been relaxed to avoid the need for numerous
concrete implementations in the standard library for traits like `Eq`
and `PartialEq`. This decision was made because support for const
generics is expected soon, which will allow these traits to be
implemented more cleanly.

Closes issue #5892.
  • Loading branch information
tritao committed Feb 21, 2025
1 parent a1651ec commit af25915
Show file tree
Hide file tree
Showing 41 changed files with 96 additions and 897 deletions.
16 changes: 12 additions & 4 deletions sway-core/src/language/ty/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
decl_engine::*,
fuel_prelude::fuel_tx::StorageSlot,
language::{parsed, ty::*, Purity, Visibility},
namespace::TraitMap,
namespace::{check_impls_for_overlap, check_orphan_rules_for_impls},
semantic_analysis::namespace,
transform::AllowDeprecatedState,
type_system::*,
Expand Down Expand Up @@ -427,7 +427,7 @@ impl TyProgram {
}

// check orphan rules for all traits
TraitMap::check_orphan_rules(handler, engines, root_namespace.root_ref())?;
check_orphan_rules_for_impls(handler, engines, root_namespace.root_ref())?;

// check trait overlap
let mut unified_trait_map = root_namespace
Expand All @@ -437,7 +437,14 @@ impl TyProgram {
.items
.implemented_traits
.clone();
unified_trait_map.check_overlap_and_extend(handler, unified_trait_map.clone(), engines)?;

let other_trait_map = unified_trait_map.clone();
check_impls_for_overlap(
&mut unified_trait_map,
handler,
other_trait_map,
engines,
)?;

for (submod_name, submodule) in root.submodules.iter() {
root_namespace.push_submodule(
Expand All @@ -448,7 +455,8 @@ impl TyProgram {
submodule.mod_name_span.clone(),
)?;

unified_trait_map.check_overlap_and_extend(
check_impls_for_overlap(
&mut unified_trait_map,
handler,
root_namespace
.current_module()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2555,7 +2555,7 @@ impl ty::TyExpression {
base_name: &Ident,
projections: &[ty::ProjectionKind],
) -> Result<(TypeId, TypeId), ErrorEmitted> {
let ret = module.walk_scope_chain(|lexical_scope| {
let ret = module.walk_scope_chain_early_return(|lexical_scope| {
Self::find_subfield_type_helper(
lexical_scope,
handler,
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/namespace/lexical_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ impl Items {
}
};

let _ = module.walk_scope_chain(|lexical_scope| {
let _ = module.walk_scope_chain_early_return(|lexical_scope| {
if let Some((ident, decl)) = lexical_scope.items.symbols.get_key_value(&name) {
append_shadowing_error(
ident,
Expand Down
3 changes: 3 additions & 0 deletions sway-core/src/semantic_analysis/namespace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod module;
#[allow(clippy::module_inception)]
mod namespace;
mod root;
mod trait_coherence;
mod trait_map;

pub use contract_helpers::*;
Expand All @@ -13,6 +14,8 @@ pub use module::Module;
pub use namespace::Namespace;
pub use root::ResolvedDeclaration;
pub use root::Root;
pub(crate) use trait_coherence::check_impls_for_overlap;
pub(crate) use trait_coherence::check_orphan_rules_for_impls;
pub(crate) use trait_map::IsExtendingExistingImpl;
pub(crate) use trait_map::IsImplSelf;
pub(super) use trait_map::ResolvedTraitImplItem;
Expand Down
19 changes: 6 additions & 13 deletions sway-core/src/semantic_analysis/namespace/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ impl Module {
self.current_lexical_scope_id = parent_scope_id.unwrap(); // panics if pops do not match pushes
}

pub fn walk_scope_chain<T>(
pub fn walk_scope_chain_early_return<T>(
&self,
mut f: impl FnMut(&LexicalScope) -> Result<Option<T>, ErrorEmitted>,
) -> Result<Option<T>, ErrorEmitted> {
Expand All @@ -310,23 +310,16 @@ impl Module {
Ok(None)
}

pub fn walk_scope_chain_mut<T>(
&mut self,
mut f: impl FnMut(&mut LexicalScope) -> Result<Option<T>, ErrorEmitted>,
) -> Result<Option<T>, ErrorEmitted> {
let mut lexical_scope_opt = Some(self.current_lexical_scope_mut());
pub fn walk_scope_chain(&self, mut f: impl FnMut(&LexicalScope)) {
let mut lexical_scope_opt = Some(self.current_lexical_scope());
while let Some(lexical_scope) = lexical_scope_opt {
let result = f(lexical_scope)?;
if let Some(result) = result {
return Ok(Some(result));
}
f(lexical_scope);
if let Some(parent_scope_id) = lexical_scope.parent {
lexical_scope_opt = self.get_lexical_scope_mut(parent_scope_id);
lexical_scope_opt = self.get_lexical_scope(parent_scope_id);
} else {
lexical_scope_opt = None;
}
}
Ok(None)
}

pub fn get_items_for_type(
Expand All @@ -344,7 +337,7 @@ impl Module {
symbol: &Ident,
) -> Result<(ResolvedDeclaration, ModulePathBuf), ErrorEmitted> {
let mut last_handler = Handler::default();
let ret = self.walk_scope_chain(|lexical_scope| {
let ret = self.walk_scope_chain_early_return(|lexical_scope| {
last_handler = Handler::default();
Ok(lexical_scope
.items
Expand Down
10 changes: 8 additions & 2 deletions sway-core/src/semantic_analysis/namespace/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,10 +909,16 @@ impl Root {
ignored_prefixes += 1;
}

println!("{:?}", ignored_prefixes);
println!("prefixes {:?}", iter_prefixes(src).collect::<Vec<_>>());
// Check visibility of remaining submodules in the source path
for prefix in iter_prefixes(src).skip(ignored_prefixes) {
if let Some(module) = self.module_from_absolute_path(&prefix.to_vec()) {
for prefix in iter_prefixes(src) {
let prefix = prefix.iter().skip(ignored_prefixes).cloned().collect::<Vec<_>>();
println!("prefix {:?}", prefix);
if let Some(module) = self.module_from_absolute_path(&prefix) {
if module.visibility().is_private() {
println!("src {:?}", src);
println!("dst {:?}", dst);
let prefix_last = prefix[prefix.len() - 1].clone();
handler.emit_err(CompileError::ImportPrivateModule {
span: prefix_last.span(),
Expand Down
Loading

0 comments on commit af25915

Please sign in to comment.