Skip to content
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.

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

Handle implicit Sized bounds #8984

Closed
flodiebold opened this issue May 25, 2021 · 7 comments · Fixed by #9807
Closed

Handle implicit Sized bounds #8984

flodiebold opened this issue May 25, 2021 · 7 comments · Fixed by #9807
Assignees
Labels
A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started E-medium S-actionable Someone could pick this issue up and work on it right now

Comments

@flodiebold
Copy link
Member

flodiebold commented May 25, 2021

We currently don't add the implicit Sized bounds to type parameters, which leads to some inaccuracies in trait solving, so we need to fix this.

Rough steps:

  • find out the exact conditions for the implicit Sized bound (is it simply all type parameters and associated types without ?Sized except trait Self? not sure)
  • during lowering of generic predicates, add the Sized bounds for all type parameters that don't have ?Sized (I feel it makes more sense to add them at this step, rather than in hir_def, but I'm not 100% sure) except for trait Self
  • similarly, add the bounds for associated types; their bounds are currently lowered directly in the Chalk integration, which we should change at some point (maybe now!)
  • make sure we don't suddenly print + Sized everywhere, e.g. when displaying argument impl Trait types
  • the coerce_unsize_apit test should now work correctly; maybe add some explicit tests with Sized bounds in trait impls
  • check analysis-stats performance before/after
@flodiebold flodiebold added A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started E-medium S-actionable Someone could pick this issue up and work on it right now labels May 25, 2021
@bjorn3
Copy link
Member

bjorn3 commented May 25, 2021

is it simply all type parameters without ?Sized except trait Self?

As far as I am aware that is the case.

@flodiebold
Copy link
Member Author

Hmm, associated types get it as well, don't they?

@bjorn3
Copy link
Member

bjorn3 commented May 25, 2021

They would have to for Self::Foo to be acceptable as argument and return type.

@iDawer
Copy link
Contributor

iDawer commented May 31, 2021

If nobody working on this yet, I'd like to claim it.

Nice opportunity to learn Chalk!

Got some intermediate results
//#[test]
//fn play() {
//    check_infer(
//        r#"
#[lang = "sized"]
trait Sized {}

trait Foo {
    fn foo() -> u8;
}
trait Trait<T: Foo + ?Sized, U: Foo> {
    fn bar() {
        T::foo;
    }
}
//"#,
//        expect_test::expect![[]],
//    );
//}

dbg!() output:

running 1 test
generic_predicates_query: def = TraitId(TraitId(1)), trait Foo
[crates/hir_ty/src/lower.rs:1096] &explicitly_unsized_tys = {}
[crates/hir_ty/src/lower.rs:1121] predicates = []
generic_predicates_query: def = TraitId(TraitId(2)), trait Trait
[crates/hir_ty/src/lower.rs:1096] &explicitly_unsized_tys = {
    ^0.1,
}
[crates/hir_ty/src/lower.rs:1121] predicates = [
    for<type, type, type> for<> Implemented(^1.1: TraitId(1)),
    for<type, type, type> for<> Implemented(^1.2: TraitId(1)),
    for<type, type, type> for<> Implemented(^1.2: TraitId(0)),
]
generic_predicates_query: def = TraitId(TraitId(0)), trait Sized
[crates/hir_ty/src/lower.rs:1096] &explicitly_unsized_tys = {}
[crates/hir_ty/src/lower.rs:1121] predicates = []

@Veykril Veykril assigned Veykril and iDawer and unassigned Veykril May 31, 2021
@jonas-schievink
Copy link
Contributor

@iDawer Are you still working on this? If not I can pick this up

@flodiebold
Copy link
Member Author

@iDawer
Copy link
Contributor

iDawer commented Jul 29, 2021

I apologize for being missing without a notify. I'm back and will continue on this

@bors bors bot closed this as completed in baf1494 Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started E-medium S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants