Skip to content

Remove dead code and re-enable functions tests #664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 18, 2021
Merged

Remove dead code and re-enable functions tests #664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed96821
Remove dead (moved) recursive solver code
AzureMarker Dec 11, 2020
32f7922
Re-enable the functions.rs tests
AzureMarker Dec 15, 2020
6368af3
Fix for functions file
jackh726 Dec 15, 2020
4c2cb04
Fix tests by floundering on general bound vars in builtin trait code
AzureMarker Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions chalk-solve/src/clauses/builtin_traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ pub fn add_builtin_program_clauses<I: Interner>(

match well_known {
WellKnownTrait::Sized => {
sized::add_sized_program_clauses(db, builder, trait_ref, ty, binders)
sized::add_sized_program_clauses(db, builder, trait_ref, ty, binders)?;
}
WellKnownTrait::Copy => {
copy::add_copy_program_clauses(db, builder, trait_ref, ty, binders)
copy::add_copy_program_clauses(db, builder, trait_ref, ty, binders)?;
}
WellKnownTrait::Clone => {
clone::add_clone_program_clauses(db, builder, trait_ref, ty, binders)
clone::add_clone_program_clauses(db, builder, trait_ref, ty, binders)?;
}
WellKnownTrait::FnOnce | WellKnownTrait::FnMut | WellKnownTrait::Fn => {
fn_family::add_fn_trait_program_clauses(db, builder, well_known, self_ty)?
Expand Down
6 changes: 3 additions & 3 deletions chalk-solve/src/clauses/builtin_traits/clone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::clauses::ClauseBuilder;
use crate::{Interner, RustIrDatabase, TraitRef};
use chalk_ir::{CanonicalVarKinds, TyKind};
use chalk_ir::{CanonicalVarKinds, Floundered, TyKind};

use super::copy::add_copy_program_clauses;

Expand All @@ -10,7 +10,7 @@ pub fn add_clone_program_clauses<I: Interner>(
trait_ref: TraitRef<I>,
ty: TyKind<I>,
binders: &CanonicalVarKinds<I>,
) {
) -> Result<(), Floundered> {
// Implement Clone for types that automaticly implement Copy
add_copy_program_clauses(db, builder, trait_ref, ty, binders);
add_copy_program_clauses(db, builder, trait_ref, ty, binders)
}
21 changes: 14 additions & 7 deletions chalk-solve/src/clauses/builtin_traits/copy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::clauses::builtin_traits::needs_impl_for_tys;
use crate::clauses::ClauseBuilder;
use crate::{Interner, RustIrDatabase, TraitRef};
use chalk_ir::{CanonicalVarKinds, Substitution, TyKind, TyVariableKind, VariableKind};
use chalk_ir::{CanonicalVarKinds, Floundered, Substitution, TyKind, TyVariableKind, VariableKind};
use std::iter;
use tracing::instrument;

Expand Down Expand Up @@ -37,7 +37,7 @@ pub fn add_copy_program_clauses<I: Interner>(
trait_ref: TraitRef<I>,
ty: TyKind<I>,
binders: &CanonicalVarKinds<I>,
) {
) -> Result<(), Floundered> {
match ty {
TyKind::Tuple(arity, ref substitution) => {
push_tuple_copy_conditions(db, builder, trait_ref, arity, substitution)
Expand Down Expand Up @@ -73,20 +73,27 @@ pub fn add_copy_program_clauses<I: Interner>(

TyKind::Function(_) => builder.push_fact(trait_ref),

TyKind::InferenceVar(_, kind) => match kind {
TyVariableKind::Integer | TyVariableKind::Float => builder.push_fact(trait_ref),
TyVariableKind::General => {}
},
TyKind::InferenceVar(_, TyVariableKind::Float)
| TyKind::InferenceVar(_, TyVariableKind::Integer) => builder.push_fact(trait_ref),

TyKind::BoundVar(bound_var) => {
let var_kind = &binders.at(db.interner(), bound_var.index).kind;
match var_kind {
VariableKind::Ty(TyVariableKind::Integer)
| VariableKind::Ty(TyVariableKind::Float) => builder.push_fact(trait_ref),
VariableKind::Ty(_) | VariableKind::Const(_) | VariableKind::Lifetime => {}

// Don't know enough
VariableKind::Ty(TyVariableKind::General) => return Err(Floundered),

VariableKind::Const(_) | VariableKind::Lifetime => {}
}
}

// Don't know enough
TyKind::InferenceVar(_, TyVariableKind::General) => return Err(Floundered),

// These should be handled elsewhere
TyKind::Alias(_) | TyKind::Dyn(_) | TyKind::Placeholder(_) => {}
};
Ok(())
}
2 changes: 1 addition & 1 deletion chalk-solve/src/clauses/builtin_traits/fn_family.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ pub fn add_fn_trait_program_clauses<I: Interner>(
Ok(())
}
// Function traits are non-enumerable
TyKind::InferenceVar(..) | TyKind::Alias(..) => Err(Floundered),
TyKind::InferenceVar(..) | TyKind::BoundVar(_) | TyKind::Alias(..) => Err(Floundered),
_ => Ok(()),
}
}
22 changes: 15 additions & 7 deletions chalk-solve/src/clauses/builtin_traits/sized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use crate::clauses::builtin_traits::needs_impl_for_tys;
use crate::clauses::ClauseBuilder;
use crate::rust_ir::AdtKind;
use crate::{Interner, RustIrDatabase, TraitRef};
use chalk_ir::{AdtId, CanonicalVarKinds, Substitution, TyKind, TyVariableKind, VariableKind};
use chalk_ir::{
AdtId, CanonicalVarKinds, Floundered, Substitution, TyKind, TyVariableKind, VariableKind,
};

fn push_adt_sized_conditions<I: Interner>(
db: &dyn RustIrDatabase<I>,
Expand Down Expand Up @@ -70,7 +72,7 @@ pub fn add_sized_program_clauses<I: Interner>(
trait_ref: TraitRef<I>,
ty: TyKind<I>,
binders: &CanonicalVarKinds<I>,
) {
) -> Result<(), Floundered> {
match ty {
TyKind::Adt(adt_id, ref substitution) => {
push_adt_sized_conditions(db, builder, trait_ref, adt_id, substitution)
Expand Down Expand Up @@ -104,13 +106,19 @@ pub fn add_sized_program_clauses<I: Interner>(
match var_kind {
VariableKind::Ty(TyVariableKind::Integer)
| VariableKind::Ty(TyVariableKind::Float) => builder.push_fact(trait_ref),
VariableKind::Ty(_) | VariableKind::Const(_) | VariableKind::Lifetime => {}

// Don't know enough
VariableKind::Ty(TyVariableKind::General) => return Err(Floundered),

VariableKind::Const(_) | VariableKind::Lifetime => {}
}
}

TyKind::InferenceVar(_, TyVariableKind::General)
| TyKind::Placeholder(_)
| TyKind::Dyn(_)
| TyKind::Alias(_) => {}
// We don't know enough here
TyKind::InferenceVar(_, TyVariableKind::General) => return Err(Floundered),

// These would be handled elsewhere
TyKind::Placeholder(_) | TyKind::Dyn(_) | TyKind::Alias(_) => {}
}
Ok(())
}
196 changes: 0 additions & 196 deletions chalk-solve/src/recursive/lib.rs
View file
Open in desktop

This file was deleted.

10 changes: 7 additions & 3 deletions tests/test/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ fn functions_are_sized() {
}
}

#[test]
fn functions_are_copy() {
test! {
program {
Expand Down Expand Up @@ -61,6 +62,9 @@ fn function_implement_fn_traits() {
struct Ty { }

trait Clone { }

impl Clone for Ty { }

opaque type MyOpaque: Clone = Ty;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last two tests in this set are supposed to flounder, but instead return "no more solutions". Maybe the floundering behavior has purposefully changed since this test was written?

Copy link
Member Author

@AzureMarker AzureMarker Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something like the following should be added here

chalk/chalk-solve/src/clauses.rs

Lines 423 to 426 in 0b391f8

TyKind::OpaqueType(opaque_ty_id, _) => {
db.opaque_ty_data(*opaque_ty_id)
.to_program_clauses(builder, environment);
} 

if /* trait does not appear in opaque type bound */ {
    return Err(Floundered);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New output looks correct here


}
Expand Down Expand Up @@ -234,11 +238,11 @@ fn function_implement_fn_traits() {
"Floundered"
}

// Tests that we flounder for alias type (opaque)
// No solution for alias type
goal {
MyOpaque: FnOnce<()>
} yields_first[SolverChoice::slg(3, None)] {
"Floundered"
} yields {
"No possible solution"
}
}
}
9 changes: 3 additions & 6 deletions tests/test/misc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,22 +538,19 @@ fn builtin_impl_enumeration() {
goal {
exists<T> { T: Copy }
} yields {
// FIXME: wrong, e.g. &u8 is also Copy
"Unique; substitution [?0 := Uint(U8)]"
"Ambiguous; no inference guidance"
}

goal {
exists<T> { T: Clone }
} yields {
// FIXME: wrong, e.g. &u8 is also Clone
"Unique; substitution [?0 := Uint(U8)]"
"Ambiguous; no inference guidance"
}

goal {
exists<T> { T: Sized }
} yields {
// FIXME: wrong, most of the built-in types are Sized
"No possible solution"
"Ambiguous; no inference guidance"
}
}
}
Expand Down
Loading