Skip to content

Commit

Permalink
Merge pull request #155 from lqd/unsafe-traits
Browse files Browse the repository at this point in the history
Implement unsafe trait support (continued)
  • Loading branch information
nikomatsakis authored Nov 12, 2023
2 parents b7fd961 + 53d0e43 commit ddbb916
Show file tree
Hide file tree
Showing 22 changed files with 161 additions and 27 deletions.
40 changes: 30 additions & 10 deletions crates/formality-check/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ use anyhow::bail;

use fn_error_context::context;
use formality_core::Downcasted;
use formality_prove::Env;
use formality_prove::{Env, Safety};
use formality_rust::{
grammar::{
AssociatedTy, AssociatedTyBoundData, AssociatedTyValue, AssociatedTyValueBoundData, Fn,
FnBoundData, ImplItem, NegTraitImpl, NegTraitImplBoundData, TraitBoundData, TraitImpl,
TraitImplBoundData, TraitItem,
FnBoundData, ImplItem, NegTraitImpl, NegTraitImplBoundData, Trait, TraitBoundData,
TraitImpl, TraitImplBoundData, TraitItem,
},
prove::ToWcs,
};
Expand All @@ -17,10 +17,8 @@ use formality_types::{
};

impl super::Check<'_> {
#[context("check_trait_impl({v:?})")]
pub(super) fn check_trait_impl(&self, v: &TraitImpl) -> Fallible<()> {
let TraitImpl { binder } = v;

#[context("check_trait_impl({trait_impl:?})")]
pub(super) fn check_trait_impl(&self, trait_impl: &TraitImpl) -> Fallible<()> {
let mut env = Env::default();

let TraitImplBoundData {
Expand All @@ -29,7 +27,7 @@ impl super::Check<'_> {
trait_parameters,
where_clauses,
impl_items,
} = env.instantiate_universally(binder);
} = env.instantiate_universally(&trait_impl.binder);

let trait_ref = trait_id.with(self_ty, trait_parameters);

Expand All @@ -45,32 +43,54 @@ impl super::Check<'_> {
trait_items,
} = trait_decl.binder.instantiate_with(&trait_ref.parameters)?;

self.check_safety_matches(&trait_decl, &trait_impl)?;

for impl_item in &impl_items {
self.check_trait_impl_item(&env, &where_clauses, &trait_items, impl_item)?;
}

Ok(())
}

pub(super) fn check_neg_trait_impl(&self, i: &NegTraitImpl) -> Fallible<()> {
#[context("check_neg_trait_impl({trait_impl:?})")]
pub(super) fn check_neg_trait_impl(&self, trait_impl: &NegTraitImpl) -> Fallible<()> {
let mut env = Env::default();

let NegTraitImplBoundData {
trait_id,
self_ty,
trait_parameters,
where_clauses,
} = env.instantiate_universally(&i.binder);
} = env.instantiate_universally(&trait_impl.binder);

let trait_ref = trait_id.with(self_ty, trait_parameters);

// Negative impls are always safe (rustc E0198) regardless of the trait's safety.
if trait_impl.safety == Safety::Unsafe {
bail!("negative impls cannot be unsafe");
}

self.prove_where_clauses_well_formed(&env, &where_clauses, &where_clauses)?;

self.prove_goal(&env, &where_clauses, trait_ref.not_implemented())?;

Ok(())
}

/// Validate that the declared safety of an impl matches the one from the trait declaration.
fn check_safety_matches(&self, trait_decl: &Trait, trait_impl: &TraitImpl) -> Fallible<()> {
if trait_decl.safety != trait_impl.safety {
match trait_decl.safety {
Safety::Safe => bail!("implementing the trait `{:?}` is not unsafe", trait_decl.id),
Safety::Unsafe => bail!(
"the trait `{:?}` requires an `unsafe impl` declaration",
trait_decl.id
),
}
}
Ok(())
}

fn check_trait_impl_item(
&self,
env: &Env,
Expand Down
6 changes: 5 additions & 1 deletion crates/formality-check/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ use formality_types::grammar::Fallible;
impl super::Check<'_> {
#[context("check_trait({:?})", t.id)]
pub(super) fn check_trait(&self, t: &Trait) -> Fallible<()> {
let Trait { id: _, binder } = t;
let Trait {
safety: _,
id: _,
binder,
} = t;
let mut env = Env::default();

let TraitBoundData {
Expand Down
6 changes: 3 additions & 3 deletions crates/formality-core/src/judgment/test_filtered.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![cfg(test)]

use std::sync::Arc;
use crate::cast_impl;

use crate::cast_impl;
use crate::judgment_fn;

#[derive(Ord, PartialOrd, Eq, PartialEq, Clone, Debug, Hash)]
Expand All @@ -24,14 +24,14 @@ impl Graph {
judgment_fn!(
fn transitive_reachable(g: Arc<Graph>, node: u32) => u32 {
debug(node, g)

(
(graph.successors(a) => b)
(if b % 2 == 0)
--------------------------------------- ("base")
(transitive_reachable(graph, a) => b)
)

(
(transitive_reachable(&graph, a) => b)
(transitive_reachable(&graph, b) => c)
Expand Down
12 changes: 11 additions & 1 deletion crates/formality-macros/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,24 @@ fn debug_variant_with_attr(

fn debug_field_with_mode(name: &Ident, mode: &FieldMode) -> TokenStream {
match mode {
FieldMode::Single | FieldMode::Optional => {
FieldMode::Single => {
quote_spanned! { name.span() =>
write!(fmt, "{}", sep)?;
write!(fmt, "{:?}", #name)?;
sep = " ";
}
}

FieldMode::Optional => {
quote_spanned! { name.span() =>
if !::formality_core::util::is_default(#name) {
write!(fmt, "{}", sep)?;
write!(fmt, "{:?}", #name)?;
sep = " ";
}
}
}

FieldMode::Many => {
quote_spanned! { name.span() =>
for e in #name {
Expand Down
1 change: 1 addition & 0 deletions crates/formality-prove/src/db.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

35 changes: 32 additions & 3 deletions crates/formality-prove/src/decls.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::fmt;

use formality_core::{set, Set, Upcast};
use formality_macros::term;
use formality_types::grammar::{
Expand Down Expand Up @@ -104,8 +106,10 @@ impl Decls {

/// An "impl decl" indicates that a trait is implemented for a given set of types.
/// One "impl decl" is created for each impl in the Rust source.
#[term(impl $binder)]
#[term($?safety impl $binder)]
pub struct ImplDecl {
/// The safety this impl declares, which needs to match the implemented trait's safety.
pub safety: Safety,
/// The binder covers the generic variables from the impl
pub binder: Binder<ImplDeclBoundData>,
}
Expand All @@ -122,8 +126,11 @@ pub struct ImplDeclBoundData {

/// A declaration that some trait will *not* be implemented for a type; derived from negative impls
/// like `impl !Foo for Bar`.
#[term(impl $binder)]
#[term($?safety impl $binder)]
pub struct NegImplDecl {
/// The safety this negative impl declares
pub safety: Safety,

/// Binder comes the generics on the impl
pub binder: Binder<NegImplDeclBoundData>,
}
Expand All @@ -135,15 +142,37 @@ pub struct NegImplDeclBoundData {
pub where_clause: Wcs,
}

/// Mark a trait or trait impl as `unsafe`.
#[term]
#[customize(debug)]
#[derive(Default)]
pub enum Safety {
#[default]
Safe,
Unsafe,
}

impl fmt::Debug for Safety {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Safety::Safe => write!(f, "safe"),
Safety::Unsafe => write!(f, "unsafe"),
}
}
}

/// A "trait declaration" declares a trait that exists, its generics, and its where-clauses.
/// It doesn't capture the trait items, which will be transformed into other sorts of rules.
///
/// In Rust syntax, it covers the `trait Foo: Bar` part of the declaration, but not what appears in the `{...}`.
#[term(trait $id $binder)]
#[term($?safety trait $id $binder)]
pub struct TraitDecl {
/// The name of the trait
pub id: TraitId,

/// Whether the trait is `unsafe` or not
pub safety: Safety,

/// The binder here captures the generics of the trait; it always begins with a `Self` type.
pub binder: Binder<TraitDeclBoundData>,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/formality-prove/src/prove/minimize/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn minimize_a() {
let (mut env_min, term_min, m) = minimize(env, term);

expect!["(Env { variables: [?ty_0, ?ty_1], coherence_mode: false }, [?ty_0, ?ty_1])"]
.assert_eq(&format!("{:?}", (&env_min, &term_min)));
.assert_eq(&format!("{:?}", (&env_min, &term_min)));

let ty0 = term_min[0].as_variable().unwrap();
let ty1 = term_min[1].as_variable().unwrap();
Expand Down
10 changes: 7 additions & 3 deletions crates/formality-rust/src/grammar.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::sync::Arc;

use formality_core::{term, Upcast};
use formality_prove::Safety;
use formality_types::{
grammar::{
AdtId, AliasTy, AssociatedItemId, Binder, Const, CrateId, Fallible, FieldId, FnId, Lt,
Expand Down Expand Up @@ -160,8 +161,9 @@ pub struct Variant {
pub fields: Vec<Field>,
}

#[term(trait $id $binder)]
#[term($?safety trait $id $binder)]
pub struct Trait {
pub safety: Safety,
pub id: TraitId,
pub binder: TraitBinder<TraitBoundData>,
}
Expand Down Expand Up @@ -241,8 +243,9 @@ pub struct AssociatedTyBoundData {
pub where_clauses: Vec<WhereClause>,
}

#[term(impl $binder)]
#[term($?safety impl $binder)]
pub struct TraitImpl {
pub safety: Safety,
pub binder: Binder<TraitImplBoundData>,
}

Expand All @@ -267,8 +270,9 @@ impl TraitImplBoundData {
}
}

#[term(impl $binder)]
#[term($?safety impl $binder)]
pub struct NegTraitImpl {
pub safety: Safety,
pub binder: Binder<NegTraitImplBoundData>,
}

Expand Down
12 changes: 8 additions & 4 deletions crates/formality-rust/src/prove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl Crate {
self.items
.iter()
.flat_map(|item| match item {
CrateItem::Trait(Trait { id, binder }) => {
CrateItem::Trait(Trait { id, binder, safety }) => {
let (
vars,
TraitBoundData {
Expand All @@ -89,6 +89,7 @@ impl Crate {
},
) = binder.open();
Some(prove::TraitDecl {
safety: safety.clone(),
id: id.clone(),
binder: Binder::new(
vars,
Expand All @@ -110,7 +111,7 @@ impl Crate {
self.items
.iter()
.flat_map(|item| match item {
CrateItem::TraitImpl(TraitImpl { binder }) => {
CrateItem::TraitImpl(TraitImpl { binder, safety }) => {
let (
vars,
TraitImplBoundData {
Expand All @@ -122,6 +123,7 @@ impl Crate {
},
) = binder.open();
Some(prove::ImplDecl {
safety: safety.clone(),
binder: Binder::new(
vars,
prove::ImplDeclBoundData {
Expand All @@ -140,7 +142,7 @@ impl Crate {
self.items
.iter()
.flat_map(|item| match item {
CrateItem::NegTraitImpl(NegTraitImpl { binder }) => {
CrateItem::NegTraitImpl(NegTraitImpl { binder, safety }) => {
let (
vars,
NegTraitImplBoundData {
Expand All @@ -151,6 +153,7 @@ impl Crate {
},
) = binder.open();
Some(prove::NegImplDecl {
safety: safety.clone(),
binder: Binder::new(
vars,
prove::NegImplDeclBoundData {
Expand All @@ -169,7 +172,7 @@ impl Crate {
self.items
.iter()
.flat_map(|item| match item {
CrateItem::TraitImpl(TraitImpl { binder }) => {
CrateItem::TraitImpl(TraitImpl { binder, safety: _ }) => {
let (
impl_vars,
TraitImplBoundData {
Expand Down Expand Up @@ -226,6 +229,7 @@ impl Crate {
.iter()
.flat_map(|item| match item {
CrateItem::Trait(Trait {
safety: _,
id: trait_id,
binder,
}) => {
Expand Down
1 change: 1 addition & 0 deletions examples/formality-eg/type_system.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

2 changes: 1 addition & 1 deletion rust-toolchain
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[toolchain]
channel = "nightly-2023-10-08"
components = [ "rustc-dev", "llvm-tools" ]
components = ["rustc-dev", "llvm-tools", "rustfmt"]
4 changes: 4 additions & 0 deletions tests/ui/decl_safety/safe_trait-negative_impl_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Error: check_neg_trait_impl(unsafe impl ! Foo for u32 {})

Caused by:
negative impls cannot be unsafe
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
crate baguette {
trait Foo {}
unsafe impl !Foo for u32 {}
}
]
7 changes: 7 additions & 0 deletions tests/ui/decl_safety/safe_trait.πŸ”¬
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//@check-pass
[
crate baguette {
safe trait Foo {}
safe impl Foo for u32 {}
}
]
4 changes: 4 additions & 0 deletions tests/ui/decl_safety/safe_trait_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Error: check_trait_impl(unsafe impl Foo for u32 { })

Caused by:
implementing the trait `Foo` is not unsafe
6 changes: 6 additions & 0 deletions tests/ui/decl_safety/safe_trait_mismatch.πŸ”¬
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
crate baguette {
trait Foo {}
unsafe impl Foo for u32 {}
}
]
7 changes: 7 additions & 0 deletions tests/ui/decl_safety/unsafe_trait-negative_impl.πŸ”¬
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//@check-pass
[
crate baguette {
unsafe trait Foo {}
impl !Foo for u32 {}
}
]
Loading

0 comments on commit ddbb916

Please sign in to comment.