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

Unsizing coercion does not normalize associated types in structs. #75899

Closed
eddyb opened this issue Aug 25, 2020 · 9 comments · Fixed by #101831
Closed

Unsizing coercion does not normalize associated types in structs. #75899

eddyb opened this issue Aug 25, 2020 · 9 comments · Fixed by #101831
Assignees
Labels
A-associated-items Area: Associated items (types, constants & functions) A-coercions Area: implicit and explicit `expr as Type` coercions A-DSTs Area: Dynamically-sized types (DSTs) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Aug 25, 2020

Example code I'd expect to work (playground):

// NOTE: this is only needed for coerce_tuple, you can remove both for a stable version.
#![feature(unsized_tuple_coercion)]

trait Trait {}
impl<T> Trait for T {}

trait Noop {
    type Assoc: ?Sized;
}
impl<T: ?Sized> Noop for T {
    type Assoc = T;
}

// Both of these work:
fn coerce<T: Trait>(x: &<T as Noop>::Assoc) -> &<dyn Trait as Noop>::Assoc { x }
fn coerce_tuple<T: Trait>(x: &(String, <T as Noop>::Assoc)) -> &(String, <dyn Trait as Noop>::Assoc) { x }

// But this doesn't:
struct NoopNewtype<T: ?Sized + Noop>(T::Assoc);
fn coerce_newtype<T: Trait>(x: &NoopNewtype<T>) -> &NoopNewtype<dyn Trait> { x }

(in case the example above seems artificial, the realistic one I've reduced this from involves using an associated trait to add Option around sized types, but not dyn Trait, since Option<dyn Trait> wouldn't itself work)


I'd expect coerce_newtype to work because <T as Noop>::Assoc: Unsize<<dyn Trait as Noop>::Assoc> should hold, after normalization to T: Unsize<dyn Trait> (which holds here because T: Trait), but nothing performs that normalization before coercion gives up entirely.

Unlike CoerceUnsized (where I've been wary of associated types in the past), there should be no weird interactions here, we should be able to largely treat the struct as a tuple (and we already ensure the type parameter being changed doesn't show up anywhere else in the struct, anyway).


The coerce and coerce_tuple examples get to normalize before asking the trait system about any CoerceUnsized or Unsized obligations - I wonder if we could just normalize trait_pred here?

match selcx.select(&obligation.with(trait_pred)) {

But we would have to be careful, because we are in a speculative operation, so we would have to use normalize_associated_types_in_as_infer_ok, like this other piece of coercion logic:

let InferOk { value: a_sig, mut obligations } =
self.normalize_associated_types_in_as_infer_ok(self.cause.span, &a_sig);

cc @mystor (author of the object-provider crate where this could be useful) @nikomatsakis

@eddyb eddyb added A-DSTs Area: Dynamically-sized types (DSTs) A-associated-items Area: Associated items (types, constants & functions) A-coercions Area: implicit and explicit `expr as Type` coercions E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Aug 25, 2020
@eddyb
Copy link
Member Author

eddyb commented Aug 25, 2020

FWIW, this can be worked around with explicit Unsize bounds in the right places but they're nightly-only and entirely unnecessary if we fix this issue.

@eddyb
Copy link
Member Author

eddyb commented Aug 25, 2020

Not sure if I should nominate this for compiler team or lang team, but let's try compiler team first.

@eddyb eddyb added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 25, 2020
@spastorino
Copy link
Member

This was discussed during today's compiler meeting removing nomination. @eddyb please feel free to keep the discussion going on Zulip or even re-nominating it if you consider a re-discussion important.

@WaffleLapkin
Copy link
Member

I'd like to work on this. I haven't worked on the compiler yet, so not sure I'll be able to fix this in a reasonable time. But since it's labelled E-easy it's worth giving a try, right? Any advice is appreciated :)

@rustbot claim

@nikomatsakis
Copy link
Contributor

@WaffleLapkin I'm not sure if that E-easy label is really accurate! That said, you could try to drop by in #wg-traits on Zulip at some point during one of my morning office hours times, if that works for you (9am-10 US Eastern Time, every day but Tuesday), and we could talk it over.

@SoniEx2
Copy link
Contributor

SoniEx2 commented Aug 19, 2022

is this the same as #50213?

@WaffleLapkin
Copy link
Member

@SoniEx2 yes, that's the same thing, unsize coercion doesn't properly consider associated types. (maybe it's time for me to return to this 👀 )

@SoniEx2
Copy link
Contributor

SoniEx2 commented Aug 19, 2022

if there's anything we can do to help, feel free to poke us; we want this for uazu/qcell#37. =^-^=

@compiler-errors
Copy link
Member

It's been some time this issue was touched, so let me take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-coercions Area: implicit and explicit `expr as Type` coercions A-DSTs Area: Dynamically-sized types (DSTs) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
6 participants