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

Tracking issue for unsized tuple coercion #42877

Open
qnighy opened this issue Jun 24, 2017 · 16 comments
Open

Tracking issue for unsized tuple coercion #42877

qnighy opened this issue Jun 24, 2017 · 16 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC I-lang-nominated Nominated for discussion during a lang team meeting. S-tracking-design-concerns Status: There are blocking design concerns. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@qnighy
Copy link
Contributor

qnighy commented Jun 24, 2017

This is a part of #18469. This is currently feature-gated behind #![feature(unsized_tuple_coercion)] to avoid insta-stability.

Related issues/PRs: #18469, #32702, #34451, #37685, #42527

This is needed for unsizing the last field in a tuple:

#![feature(unsized_tuple_coercion)]
fn main() {
    let _: &(i32, [i32]) = &(3, [3, 3]) as _;
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=5f93d1d7c6ee0e969df53c13e1aa941a

@Mark-Simulacrum Mark-Simulacrum added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jun 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Jul 22, 2017
@steveklabnik
Copy link
Member

Triage: no changes

@joshtriplett joshtriplett added the S-tracking-design-concerns Status: There are blocking design concerns. label Jan 26, 2022
@Mark-Simulacrum Mark-Simulacrum added the S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. label Jan 26, 2022
@Mark-Simulacrum
Copy link
Member

T-lang discussed this in a backlog bonanza meeting today, and noted several concerns that should be addressed prior to moving forward:

  • Is there active usage/desire for this feature?
  • Tuple destructuring and/or combining may conflict with particular decisions made here (for a subset of types) -- which merits at least a more thorough exploration to make sure this concern is assessed (it may not be fully accurate).

@dhardy
Copy link
Contributor

dhardy commented Jan 27, 2022

I subscribed to this because I wanted to use it once... but didn't because it's unstable and not too hard to work around. I suspect the same goes for many non-trivial projects. Searching GitHub shows 1391 code results, most of which appear to be tests. There are a few uses on the first page: https://github.com/aschaeffer/inexor-rgf-application, https://github.com/aschaeffer/rust-ecs-poc, https://github.com/libdither/disp, https://github.com/icostin/halfbit (feature = "nightly"); everything after that appears to be tests (I checked 10 more pages).

In short: some possible utility, but not a priority.

@mleonhard
Copy link

I wish to use this in safina-executor library. The library currently uses this type for async tasks:

Arc<Mutex<dyn Future<Output = ()> + Send + Unpin>>

(v0.3.2 lib.rs:306)

I am modifying it to remember when a task has returned Polling::Ready and avoid polling it again.

I would like to use the following type, but it depends on this unsized_tuple_coercion feature:

Arc<Mutex<(bool, dyn Future<Output = ()> + Send + Unpin)>>

Instead, I will use this type, which requires two allocations per task:

Arc<Mutex<Option<Box<dyn Future<Output = ()> + Send + Unpin>>>>

This is OK. Async tasks are generally long-lived data structures. Therefore, the extra allocation is a very small added cost.

This Option version has a benefit: it lets us drop the completed future right away. With the bool version, the future drops after its last waker is dropped. Some applications may use timeouts that keep a waker for the duration of the timeout. That could keep future enclosures around for minutes, and increase memory pressure.

@SimonSapin
Copy link
Contributor

To use bool on stable, does using a struct type instead of a tuple work?

@mleonhard
Copy link

@SimonSapin A struct works!

struct Task<T: ?Sized> {
    completed: bool,
    fut: T,
}
impl<T> Task<T> {
    pub fn new(fut: T) -> Arc<Mutex<Self>> {
        Arc::new(Mutex::new(Task {
            completed: false,
            fut,
        }))
    }
}

impl Executor {
//...
    pub fn spawn_unpin(self: &Arc<Self>, fut: impl (Future<Output = ()>) + Send + Unpin + 'static) {
        let task = Task::new(fut);
        let weak_self = Arc::downgrade(self);
        self.async_pool.schedule(move || poll_task(task, weak_self));
    }
}

I think I'm going to keep the Option<Box<_>> version because of the memory savings I mentioned above. Using an enum didn't work:

enum Task<T: ?Sized> {
    None,
    Some(T),
}
// error[E0277]: the size for values of type `T` cannot be known at compilation time

@mleonhard
Copy link

I just realized that the None and Some enum variants must be the same size. So the enum version wouldn't save memory used directly by the Future. It would save only heap-allocated memory owned by the Future. A completed future from an async closure cannot contain own any heap-allocated memory because it drops all variables before returning and completing. Other types that implement Future may have heap-allocated memory, but those types are rarely used as tasks. Therefore, an enum alone provides almost no memory savings because it saves only heap-allocated memory, and completed tasks have none of that.

@QuineDot
Copy link

There's an argument to be made for consistency. You can unsize coerce, deconstruct, and otherwise match against custom tuple types already. You can also do all these things on stable with built-in tuples too, given a touch of unsafe. 1

Footnotes

  1. I do recognize that's still unsound without offset_of. On the other hand, there's no other reasonable layout in this case.

@dhardy
Copy link
Contributor

dhardy commented Feb 15, 2023

@QuineDot no you can't. You are merely using references to unsized types.

However, this edited version works (avoids deconstruction of the unsized parameter, or more precisely merely avoids creating a let binding to the unsized part).

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 26, 2025

@rustbot label +I-lang-nominated

We discussed this some in the Rust-for-Linux sync meeting. This is impacting stabilization of #[derive(CoercePointee)]. There doesn't seem to be a strong rationale in favor of the feature (the argument for is essentially "why not, it works for structs") and there are some plausible responses ("so we have more freedom to play with tuples"). Therefore my inclination is to say "let's just remove this unstable feature" (we also considered trying to stabilize it, but it doesn't seem worth it to me).

It's also a 2-way door, of course, we can always add it back.

Nominating to give lang a heads up about this.

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Feb 26, 2025
@tmandry
Copy link
Member

tmandry commented Feb 27, 2025

I think the feature should exist but I don't see a reason to prioritize it. Removing it to unblock higher priority work is fine with me.

@RalfJung
Copy link
Member

@nikomatsakis

It's also a 2-way door, of course, we can always add it back.

We cannot, though. Once CoercePointee is stable, the same issue that led to this discussion will mean we cannot have unstable unsizing coercions any more. Or did I misunderstand something?

@Darksonn
Copy link
Contributor

Darksonn commented Feb 27, 2025

I also don't think this feature really pulls its own weight.

For structs, you are only forced to lay out the last field at the end when the last field is a generic parameter that is ?Sized. Those conditions pretty much coincide with exactly the structs where you want to make use of unsizing coercions. However, tuples are always forced to apply this constraint to the layout just because this feature exists on unstable Rust, just because there could be a crate in the crate-graph that uses #![feature(unsized_tuple_coercion)].

Besides, unsizing tuples has seen basically no use since it was added in 2017. It seems unlikely that anyone will push this to be stabilized any time soon.

@RalfJung
Copy link
Member

I tend to agree that we've been doing fine without tuple unsizing; unsized types aren't even that common and defining your own ADT for them isn't an undue burden -- given, as you say, that this unlocks more layout optimizations for tuples.

However, we could want other unstable unsizing coercions in the future, and it seems we are closing the door for that.

@Darksonn
Copy link
Contributor

Darksonn commented Feb 27, 2025

Niko mentioned in the meeting that this approach might be able to unblock future unstable impls. Though I tend to agree with CE's point that keeping around logic for doing unstable impls in one place and having various extra complexity to avoid leaking it seems like a bad state to be in.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 27, 2025
Remove unsizing coercions for tuples

See rust-lang#42877 (comment) and below comments for justification.

Tracking issue: rust-lang#42877
Fixes: rust-lang#135217
@lcnr
Copy link
Contributor

lcnr commented Feb 28, 2025

I am in favor of just removing this feature. Tuple unsizing coercions inhibits future layout optimizations for tuples:

The last field of tuples would always have to be last in the actual layout as any tuple may participate in unsizing.

// the actual layout of the tuple must not be `(u16, u16, u32)` as (u16, u32, dyn Send)`
// needs `dyn Send` to be at the end of the tuple.
let x: &(u16, u32, u16) = &(1, 2, 1);
let y: &(u16, u32, dyn Send) = x;

This is not necessary for size optimizations as that optimization can always keep the last field in place; it doesn't matter where in the struct we pair values with matching alignment. It would inhibit other optimizations, e.g. @m-ou-se mentioned

if you sort them by size from big to small you can avoid padding bytes in the middle though, moving them all to the end

It may also inhibit optimizations based on the "preferred alignment".

For struct unsizing this only inhibits these optimizations for structs which can be unsized (have a ?Sized param in the last field), but given that all tuples may get unsized, we would be forced to limit these optimization for all tuples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC I-lang-nominated Nominated for discussion during a lang team meeting. S-tracking-design-concerns Status: There are blocking design concerns. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests