remove no-op 'let _ = ' #105243

RalfJung · 2022-12-04T09:35:06Z

Also see the discussion at #93563 (comment).

I don't know why these Drop implementations exist to begin with, given that their body does literally nothing, but did not want to change that. (It might affect dropck.)

Cc @ibraheemdev @Amanieu

rustbot · 2022-12-04T09:35:11Z

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2022-12-04T09:35:13Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

Amanieu · 2022-12-04T09:38:26Z

I think we can remove these drop impls entirely. Any dropck requirements are handled by the inner drop.

RalfJung · 2022-12-04T10:35:58Z

The one concern raised by @ibraheemdev is that this is technically a breaking change -- in case anyone relied on Receiver<T>: Drop (which is a possible but rather silly bound one could use).

Noratrieb · 2022-12-04T12:36:24Z

If you're relying on Drop bounds then you're doing something really wrong and I don't think we should support that.
There should be a lint of : Drop bounds if there isn't already.

kpreid · 2022-12-04T18:00:24Z

There should be a lint of : Drop bounds if there isn't already.

There is: drop_bounds

Mark-Simulacrum · 2022-12-05T02:55:31Z

r=me, but no opinion on whether to remove the Drop impls too. It does seem relatively harmless.

RalfJung · 2022-12-05T11:40:20Z

All right, I have removed the empty drop impls.

@bors r=Mark-Simulacrum

bors · 2022-12-05T11:40:22Z

📌 Commit 9e05798e24e46f9c3c9f7f27d75d388497a27078 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

RalfJung · 2022-12-05T12:06:59Z

@bors r-

RalfJung · 2022-12-05T12:07:44Z

Turns out this actually makes some test fail... I guess not then. ;)

RalfJung · 2022-12-05T12:08:16Z

@bors r=Mark-Simulacrum

bors · 2022-12-05T12:08:18Z

📌 Commit c823dfa has been approved by Mark-Simulacrum

It is now in the queue for this repository.

remove no-op 'let _ = ' Also see the discussion at rust-lang#93563 (comment). I don't know why these `Drop` implementations exist to begin with, given that their body does literally nothing, but did not want to change that. (It might affect dropck.) Cc `@ibraheemdev` `@Amanieu`

remove no-op 'let _ = ' Also see the discussion at rust-lang#93563 (comment). I don't know why these `Drop` implementations exist to begin with, given that their body does literally nothing, but did not want to change that. (It might affect dropck.) Cc ``@ibraheemdev`` ``@Amanieu``

remove no-op 'let _ = ' Also see the discussion at rust-lang#93563 (comment). I don't know why these `Drop` implementations exist to begin with, given that their body does literally nothing, but did not want to change that. (It might affect dropck.) Cc ```@ibraheemdev``` ```@Amanieu```

remove no-op 'let _ = ' Also see the discussion at rust-lang#93563 (comment). I don't know why these `Drop` implementations exist to begin with, given that their body does literally nothing, but did not want to change that. (It might affect dropck.) Cc ````@ibraheemdev```` ````@Amanieu````

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#104439 (Add prototype to generate `COPYRIGHT` from REUSE metadata) - rust-lang#105005 (On E0195 point at where clause lifetime bounds) - rust-lang#105098 (propagate the error from parsing enum variant to the parser and emit out) - rust-lang#105243 (remove no-op 'let _ = ') - rust-lang#105254 (Recurse into nested impl-trait when computing variance.) - rust-lang#105287 (Synthesize substitutions for bad auto traits in dyn types) - rust-lang#105310 (Be more careful about unresolved exprs in suggestion) - rust-lang#105318 (Make `get_impl_future_output_ty` work with AFIT) - rust-lang#105339 (support `ConstKind::Expr` in `is_const_evaluatable` and `WfPredicates::compute`) - rust-lang#105340 (Avoid ICE by accounting for missing type) - rust-lang#105342 (Make `note_obligation_cause_code` take a `impl ToPredicate` for predicate) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Remove Drop impl of mpsc Receiver and (Sync)Sender This change removes the empty `Drop` implementations for `mpsc::Receiver`, `mpsc::Sender` and `mpsc::SyncSender`. These implementations do not specify `#[may_dangle]`, so by removing them we make `mpsc` types play nice with drop check. This was previously attempted in [rust-lang#105243](rust-lang#105243 (comment)) but then [abandoned due to a test failure](rust-lang#105243 (comment)). I've aligned the test with those for `Mutex` and `RwLock`.

Rollup merge of rust-lang#114965 - benschulz:mpsc-drop, r=dtolnay Remove Drop impl of mpsc Receiver and (Sync)Sender This change removes the empty `Drop` implementations for `mpsc::Receiver`, `mpsc::Sender` and `mpsc::SyncSender`. These implementations do not specify `#[may_dangle]`, so by removing them we make `mpsc` types play nice with drop check. This was previously attempted in [rust-lang#105243](rust-lang#105243 (comment)) but then [abandoned due to a test failure](rust-lang#105243 (comment)). I've aligned the test with those for `Mutex` and `RwLock`.

remove no-op 'let _ = '

c823dfa

rustbot assigned Mark-Simulacrum Dec 4, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 4, 2022

RalfJung mentioned this pull request Dec 4, 2022

Merge crossbeam-channel into std::sync::mpsc #93563

Merged

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2022

This comment has been minimized.

# to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 5, 2022

RalfJung force-pushed the no-op-let branch from 9e05798 to c823dfa Compare December 5, 2022 12:08

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2022

matthiaskrgr mentioned this pull request Dec 5, 2022

Rollup of 12 pull requests #105327

Closed

matthiaskrgr mentioned this pull request Dec 5, 2022

Rollup of 11 pull requests #105331

Closed

matthiaskrgr mentioned this pull request Dec 5, 2022

Rollup of 10 pull requests #105335

Closed

matthiaskrgr mentioned this pull request Dec 6, 2022

Rollup of 11 pull requests #105360

Closed

matthiaskrgr mentioned this pull request Dec 6, 2022

Rollup of 11 pull requests #105365

Merged

bors merged commit 7d8e329 into rust-lang:master Dec 6, 2022

rustbot added this to the 1.67.0 milestone Dec 6, 2022

RalfJung deleted the no-op-let branch December 8, 2022 21:36

benschulz mentioned this pull request Aug 18, 2023

Remove Drop impl of mpsc Receiver and (Sync)Sender #114965

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove no-op 'let _ = ' #105243

remove no-op 'let _ = ' #105243

RalfJung commented Dec 4, 2022 •

edited

Loading

rustbot commented Dec 4, 2022

rustbot commented Dec 4, 2022

Amanieu commented Dec 4, 2022

RalfJung commented Dec 4, 2022 •

edited

Loading

Noratrieb commented Dec 4, 2022

kpreid commented Dec 4, 2022

Mark-Simulacrum commented Dec 5, 2022

RalfJung commented Dec 5, 2022

bors commented Dec 5, 2022

This comment has been minimized.

RalfJung commented Dec 5, 2022

RalfJung commented Dec 5, 2022

RalfJung commented Dec 5, 2022

bors commented Dec 5, 2022

remove no-op 'let _ = ' #105243

remove no-op 'let _ = ' #105243

Conversation

RalfJung commented Dec 4, 2022 • edited Loading

rustbot commented Dec 4, 2022

rustbot commented Dec 4, 2022

Amanieu commented Dec 4, 2022

RalfJung commented Dec 4, 2022 • edited Loading

Noratrieb commented Dec 4, 2022

kpreid commented Dec 4, 2022

Mark-Simulacrum commented Dec 5, 2022

RalfJung commented Dec 5, 2022

bors commented Dec 5, 2022

This comment has been minimized.

RalfJung commented Dec 5, 2022

RalfJung commented Dec 5, 2022

RalfJung commented Dec 5, 2022

bors commented Dec 5, 2022

RalfJung commented Dec 4, 2022 •

edited

Loading

RalfJung commented Dec 4, 2022 •

edited

Loading