Add the `#[manually_drop]` attribute. #101586

ssbr · 2022-09-08T19:51:28Z

This attribute is equivalent to #[lang = "manually_drop"], and the behavior of both are extended to allow the type to define Drop, in which case, that will still be run. Only the field drop glue is suppressed.

This PR should essentially fully implement #100344, and is gated behind the feature flag manually_drop_attr.

Some additional notes:

The meaning of `#[manually_drop]`

#[manually_drop] means "do not destroy the fields automatically during destruction". ManuallyDrop, could "just", morally speaking, be #[manually_drop] struct ManuallyDrop<T>(T);.

The intent is, per the MCP, to allow customization of the drop order, without making the interface for initializing or accessing the fields of the struct clumsier than a normal Rust type. It also -- and people did seem excited about this part -- lifts ManuallyDrop from being a special language supported feature to just another user of #[manually_drop].

Insignificant Drop

There is the question of how this interacts with "insignificant" drop. I had trouble understanding the comments, but insignificant drop appears to just be a static analysis tool, and not something that changes behavior. (For example, it's used to detect if a language change will reorder drops in a meaningful way -- meaning, reorder the significant drops, not the insignificant ones.) Since it's unlikely to be used for #[manually_drop] types, I don't think it matters a whole lot. And where a destructor is defined, it would seem to make sense for #[manually_drop] types to match exactly the behavior of union, since they both have the shared property that field drop glue is suppressed.

Users that expect `adt.is_manually_drop()` <-> "is `std::mem::ManuallyDrop`"

I looked for all locations that queried for is_manually_drop in any form, and found two difficult cases which are hardcoded for ManuallyDrop in particular.

The first is a clippy lint for redundant clones. I don't understand why it special-cases ManuallyDrop, and it's almost certainly wrong for it to special-case #[manually_drop] types in general. However, without understanding the original rationale, I have trouble figuring the right fix. Perhaps it should simply be deleted altogether.

The second is unions -- specifically, the logic for disabling DerefMut. my_union.x.y = z will automatically dereference my_union.x if it implements DerefMut, unless it is ManuallyDrop, in which case it will not. This is because ManuallyDrop is a pointer back to its content, and so this will automatically call drop on a probably uninitialized field, and is unsafe.

This is true of ManuallyDrop, but not necessarily any other #[manually_drop] type. I believe the correct fix would, instead, be a way to mark and detect types which are a smart pointer whose pointee is within self. See, for example, this playground link:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=76fb22a6214ce453538fc18ec35a839d

But that needs to wait for a separate RFC. For now, we apply exactly the same restriction for ManuallyDrop and for any other #[manually_drop] type, even though it may be confusing.

To-do in future PRs

(Optional) Fix the redundant clone check to correctly handle #[manually_drop] structs that aren't ManuallyDrop.
When there is more experience with the feature (e.g. in Crubit) create a full RFC based on the MCP, and go through the remainder of the stabilization process. (Also, do things like generalize the union error messages to not specifically mention ManuallyDrop, but also mention #[manually_drop]. For as long as the feature is unstable, the error messages would be confusing if they referenced it...)

rustbot · 2022-09-08T19:51:31Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rust-highfive · 2022-09-08T19:51:31Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) soon.

Please see the contribution instructions for more information.

programmerjake · 2022-09-09T00:37:41Z

afaict ManuallyDrop would additionally need to be repr(transparent)

ssbr · 2022-09-09T02:21:18Z

afaict ManuallyDrop would additionally need to be repr(transparent)

Yes indeed, I'm exaggerating slightly in the PR description. Added a "morally" to fix.

compiler-errors · 2022-09-23T02:00:28Z

Sorry for taking so long to get to this, but I don't have the capacity to review this change right now.

r? compiler

bors · 2022-09-27T13:57:32Z

☔ The latest upstream changes (presumably #102306) made this pull request unmergeable. Please resolve the merge conflicts.

estebank · 2022-09-27T14:51:18Z

I'm going through the code, but isn't this something that shouldn't be insta-stable?

CC @rust-lang/lang

ssbr · 2022-09-27T20:27:34Z

I'm going through the code, but isn't this something that shouldn't be insta-stable?

Uh, definitely the intent was to gate this behind a feature flag. See e.g. src/test/ui/manually_drop_attr/feature-gate-manually_drop_attr.rs.

I guess I should be clearer about this in the PR description. I'll edit that in now.

estebank · 2022-11-10T20:58:28Z

Uh, definitely the intent was to gate this behind a feature flag. See e.g. src/test/ui/manually_drop_attr/feature-gate-manually_drop_attr.rs.

Shouldn't the feature flag be defined in accepted and not in active?

https://github.com/rust-lang/rust/pull/101586/files#diff-a82bdbaaa3c79096a30755dce8896aaf2885128a7f3f3a290072b767f05b6095R435-R436

estebank · 2022-11-10T21:04:47Z

compiler/rustc_typeck/src/check/place_op.rs

+                        // FIXME: This is not correct for `#[manually_drop]` in general, only when the struct is
+                        // a smart pointer whose pointee is at the same address, and whose pointee implements `Drop`.
+                        // Instead of checking for `#[manually_drop]`, this should likely be a more restricted check
+                        // for such types, or else union really should special-case and only permit `ManuallyDrop`, and
+                        // not `#[manually_drop]` types in general.


Should this be addressed before landing?

Before stabilization, but I would argue it's safe to publish an initial version (marked experimental and unstable) without making a choice here. The current behavior is "merely" overly conservative, and does not prevent (safe) experimentation with the feature.

The current behavior could even be acceptable in the final version, it's just not completely ideal.

estebank · 2022-11-10T21:06:35Z

src/tools/clippy/clippy_lints/src/redundant_clone.rs

@@ -134,6 +134,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
            }

            if let ty::Adt(def, _) = arg_ty.kind() {
+                // FIXME: This is not correct with `#[manually_drop]`, as that is just like any other type.


Should this instead be changed to check against the lang_item DefId then?

We could do that, but then we're committing to ManuallyDrop remaining special, right? (And this e.g. prohibits removing the lang item)

estebank · 2022-11-10T21:08:10Z

The code changes seem reasonable (but incomplete, given the FIXMEs?).

Apologies for the delay. Rerolling for a final reviewer as I'm a bit overwhelmed.

r? compiler

lcnr

can you remove the manually_drop lang item in this PR as well/change it to #[cfg(bootstrap)]?

compiler/rustc_middle/src/ty/adt.rs

lcnr · 2022-11-11T13:25:15Z

compiler/rustc_passes/src/check_attr.rs

@@ -2141,6 +2153,7 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) {
        sym::rustc_main,
        sym::unix_sigpipe,
        sym::derive,
+        sym::manually_drop,


i don't think this should be necessary 🤔

what happens if you don't add it here and use manually_drop as a crate attr?

You're right! That's simple enough. It's already checked by the other changes to this file, so removing it from this list makes the error messages less redundant.

ssbr · 2022-11-14T19:01:33Z

Shouldn't the feature flag be defined in accepted and not in active?

Other way around, I think -- see the dev guide, which says accepted is for stabilized features.

ssbr · 2022-11-14T20:21:48Z

can you remove the manually_drop lang item in this PR as well/change it to #[cfg(bootstrap)]?

I've done this in the version I'm going to upload, but it seems very fragile.

...
#[cfg_attr(bootstrap, lang = "manually_drop")]
#[cfg_attr(not(bootstrap), manually_drop)]
...
pub struct ManuallyDrop<T: ?Sized> { ... }

The way it's done here, updating the bootstrap version past the version that deletes the lang item version is a breaking change that requires updating this code. But if we support the lang item and the attribute both at the same time, we can delete the lang item later at our leisure, once the bootstrap rustc supports the attribute.

rustbot · 2022-11-14T20:34:22Z

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

lcnr · 2022-11-15T10:47:34Z

can you remove the manually_drop lang item in this PR as well/change it to #[cfg(bootstrap)]?

I've done this in the version I'm going to upload, but it seems very fragile.
...
#[cfg_attr(bootstrap, lang = "manually_drop")]
#[cfg_attr(not(bootstrap), manually_drop)]
...
pub struct ManuallyDrop<T: ?Sized> { ... }
The way it's done here, updating the bootstrap version past the version that deletes the lang item version is a breaking change that requires updating this code. But if we support the lang item and the attribute both at the same time, we can delete the lang item later at our leisure, once the bootstrap rustc supports the attribute.

When updating the bootstrap compiler, the person doing that goes through all cfg_attr(bootstrap, ...) annotations and removes them/changes them to always apply. This approach works quite well

ssbr · 2022-11-15T15:53:08Z

When updating the bootstrap compiler, the person doing that goes through all cfg_attr(bootstrap, ...) annotations and removes them/changes them to always apply. This approach works quite well

Ah, okay, that's good then! I was worried I'd be adding responsibilities onto someone who didn't already do this kind of thing.

LMK if I implemented it incorrectly. :)

lcnr

looks good enough to land as unstable 👍

r=me after the test changes

src/test/ui/manually_drop_attr/manually_drop-nodestructor.rs

src/test/ui/manually_drop_attr/manually_drop-destructor.rs

ssbr · 2022-11-15T22:53:25Z

OK, addressed those last two comments and also did a git pull --rebase so that merging in the github UI is possible.

Thank you for the reviews!

Add the `#[manually_drop]` attribute. This attribute is equivalent to `#[lang = "manually_drop"]`, and the behavior of both are extended to allow the type to define `Drop`, in which case, that will still be run. Only the field drop glue is suppressed. This PR should essentially fully implement rust-lang#100344, and is gated behind the feature flag `manually_drop_attr`. Some additional notes: ### The meaning of `#[manually_drop]` `#[manually_drop]` means "do not destroy the fields automatically during destruction". `ManuallyDrop`, could "just", morally speaking, be `#[manually_drop] struct ManuallyDrop<T>(T);`. The intent is, per the MCP, to allow customization of the drop order, without making the interface for initializing or accessing the fields of the struct clumsier than a normal Rust type. It also -- and people did seem excited about this part -- lifts `ManuallyDrop` from being a special language supported feature to just another user of `#[manually_drop]`. ### Insignificant Drop There is the question of how this interacts with "insignificant" drop. I had trouble understanding the comments, but insignificant drop appears to just be a static analysis tool, and not something that changes behavior. (For example, it's used to detect if a language change will reorder drops in a meaningful way -- meaning, reorder the significant drops, not the insignificant ones.) Since it's unlikely to be used for `#[manually_drop]` types, I don't think it matters a whole lot. And where a destructor is defined, it would seem to make sense for `#[manually_drop]` types to match exactly the behavior of `union`, since they both have the shared property that field drop glue is suppressed. ### Users that expect `adt.is_manually_drop()` <-> "is `std::mem::ManuallyDrop`" I looked for all locations that queried for `is_manually_drop` in any form, and found two difficult cases which are hardcoded for `ManuallyDrop` in particular. The first is a clippy lint for redundant clones. I don't understand why it special-cases `ManuallyDrop`, and it's almost certainly wrong for it to special-case `#[manually_drop]` types in general. However, without understanding the original rationale, I have trouble figuring the right fix. Perhaps it should simply be deleted altogether. The second is unions -- specifically, the logic for disabling `DerefMut`. `my_union.x.y = z` will automatically dereference `my_union.x` if it implements `DerefMut`, unless it is `ManuallyDrop`, in which case it will not. This is because `ManuallyDrop` is a pointer back to its content, and so this will automatically call `drop` on a probably uninitialized field, and is unsafe. This is true of `ManuallyDrop`, but not necessarily any other `#[manually_drop]` type. I believe the correct fix would, instead, be a way to mark and detect types which are a smart pointer whose pointee is within `self`. See, for example, this playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=76fb22a6214ce453538fc18ec35a839d But that needs to wait for a separate RFC. For now, we apply exactly the same restriction for `ManuallyDrop` and for any other `#[manually_drop]` type, even though it may be confusing. ## To-do in future PRs 1. Delete `#[lang = "manually_drop"]`. I'm not sure if anything special needs to be done here other than replacing it with `#[manually_drop]` -- is there a compatibility guarantee that must be upheld? 2. (Optional) Fix the redundant clone check to correctly handle `#[manually_drop]` structs that aren't `ManuallyDrop`. 3. When there is more experience with the feature (e.g. in Crubit) create a full RFC based on the MCP, and go through the remainder of the stabilization process. (Also, do things like generalize the union error messages to not specifically mention `ManuallyDrop`, but also mention `#[manually_drop]`. For as long as the feature is unstable, the error messages would be confusing if they referenced it...)

matthiaskrgr · 2022-11-16T14:38:54Z

@bors r- failed in a rollup
https://github.com/rust-lang-ci/rust/actions/runs/3480077401/jobs/5819419772#step:25:16485

#104489

ssbr · 2022-11-16T22:55:40Z

Sorry about that, I really had thought I'd run tests.

Fix committed and pushed.

ssbr · 2022-12-07T22:10:50Z

Is there anything I need to do here?

lcnr · 2022-12-08T12:13:56Z

To deal with the currently broken tests, you can use:

git fetch <rust-lang-branch>
git rebase <rust-lang-branch>/master
./x.py test src/test/ui --bless

flip1995 · 2022-12-30T16:33:58Z

src/tools/clippy/clippy_lints/src/redundant_clone.rs

@@ -113,6 +113,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
            }

            if let ty::Adt(def, _) = arg_ty.kind() {
+                // FIXME: This is not correct with `#[manually_drop]`, as that is just like any other type.


If this should get merged like this, could you open up a Clippy issue describing the possible extension that could be done here, please?

Dylan-DPC · 2023-01-25T06:47:23Z

@ssbr any updates on this?

This attribute is equivalent to `#[lang = "manually_drop"]`, and the behavior of both are extended to allow the type to define `Drop`, in which case, that will still be run. Only the field drop glue is suppressed. This PR should essentially fully implement rust-lang#100344. Some additional notes: `#[manually_drop]` means "do not destroy the fields automatically during destruction". `ManuallyDrop`, is (or could be) "just" `#[manually_drop] struct ManuallyDrop<T>(T);`. The intent is, per the MCP, to allow customization of the drop order, without making the interface for initializing or accessing the fields of the struct clumsier than a normal Rust type. It also -- and people did seem excited about this part -- lifts `ManuallyDrop` from being a special language supported feature to just another user of `#[manually_drop]`. There is the question of how this interacts with "insignificant" drop. I had trouble understanding the comments, but insignificant drop appears to just be a static analysis tool, and not something that changes behavior. (For example, it's used to detect if a language change will reorder drops in a meaningful way -- meaning, reorder the significant drops, not the insignificant ones.) Since it's unlikely to be used for `#[manually_drop]` types, I don't think it matters a whole lot. And where a destructor is defined, it would seem to make sense for `#[manually_drop]` types to match exactly the behavior of `union`, since they both have the shared property that field drop glue is suppressed. I looked for all locations that queried for `is_manually_drop` in any form, and found two difficult cases which are hardcoded for `ManuallyDrop` in particular. The first is a clippy lint for redundant clones. I don't understand why it special-cases `ManuallyDrop`, and it's almost certainly wrong for it to special-case `#[manually_drop]` types in general. However, without understanding the original rationale, I have trouble figuring the right fix. Perhaps it should simply be deleted altogether. The second is unions -- specifically, the logic for disabling `DerefMut`. `my_union.x.y = z` will automatically dereference `my_union.x` if it implements `DerefMut`, unless it is `ManuallyDrop`, in which case it will not. This is because `ManuallyDrop` is a pointer back to its content, and so this will automatically call `drop` on a probably uninitialized field, and is unsafe. This is true of `ManuallyDrop`, but not necessarily any other `#[manually_drop]` type. I believe the correct fix would, instead, be a way to mark and detect types which are a smart pointer whose pointee is within `self`. See, for example, this playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=76fb22a6214ce453538fc18ec35a839d But that needs to wait for a separate RFC. For now, we apply exactly the same restriction for `ManuallyDrop` and for any other `#[manually_drop]` type, even though it may be confusing. 1. Delete `#[lang = "manually_drop"]`. I'm not sure if anything special needs to be done here other than replacing it with `#[manually_drop]` -- is there a compatibility guarantee that must be upheld? 2. (Optional) Fix the redundant clone check to correctly handle `#[manually_drop]` structs that aren't `ManuallyDrop`. 3. When there is more experience with the feature (e.g. in Crubit) create a full RFC based on the MCP, and go through the remainder of the stabilization process. (Also, do things like generalize the union error messages to not specifically mention `ManuallyDrop`, but also mention `#[manually_drop]`. For as long as the feature is unstable, the error messages would be confusing if they referenced it...)

Co-authored-by: lcnr <rust@lcnr.de>

... honestly, I'd thought I'd fixed this and tested before, this was my bad. All it took was `./x.py test src/test/ui --bless` :(

ssbr · 2023-02-06T22:22:51Z

@ssbr any updates on this?

Sorry, I was really confused about the AI because the tests continue to all pass on my side, and then I let it drop from my radar. Let me take a look at how to reproduce, so that I can --bless.

rust-log-analyzer · 2023-02-08T09:37:28Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:ac593985615ec2ede58e132d2e21d2b1cbd6127c)
Download action repository 'rust-lang/simpleinfra@master' (SHA:2efbfac82b677cf89dacd548a824f7377c8d4e77)
Complete job name: PR (x86_64-gnu-tools, false, 1, ubuntu-20.04-xl)
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: x86_64-gnu-tools
---
  IMAGE: x86_64-gnu-tools
##[endgroup]
From https://github.com/rust-lang/rust
 * branch              master     -> FETCH_HEAD
Searching for toolstate changes between 5dd0e1b7ae7bcddce28658487602e8a077737a3e and 990b4b6cd06bcf275b4d690e8425013fbe18cb8c
Tool subtrees were updated
##[group]Run src/ci/scripts/verify-channel.sh
src/ci/scripts/verify-channel.sh
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
---

-error: the inner value of this ManuallyDrop will not be dropped
-  --> $DIR/undropped_manually_drops.rs:14:5
-   |
-LL |     drop(std::mem::ManuallyDrop::new(S));
-   |
-   |
-   = help: to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop
-   = note: `-D clippy::undropped-manually-drops` implied by `-D warnings`
-error: the inner value of this ManuallyDrop will not be dropped
error: test failed, to rerun pass `--test compile-test`
error: test failed, to rerun pass `--test compile-test`
+error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes
    |
 LL |     drop(manual1);
    |     ^^^^^^^^^^^^^
    |
    |
-   = help: to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop
+note: argument has type `std::mem::ManuallyDrop<S>`
+   |
+LL |     drop(manual1);
+   |          ^^^^^^^
+   |          ^^^^^^^
+   = note: `-D clippy::drop-non-drop` implied by `-D warnings`
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 
 
---
To only update this specific test, also pass `--test-args undropped_manually_drops.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/clippy-driver" "tests/ui/undropped_manually_drops.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/test/ui/undropped_manually_drops.stage-id" "-A" "unused" "--emit=metadata" "-Dwarnings" "-Zui-testing" "-L" "dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps" "-L" "dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps" "--extern" "if_chain=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libif_chain-03f75cdc6d4d3afc.rlib" "--extern" "itertools=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libitertools-b6f83e8bf7b1d2e3.rlib" "--extern" "clippy_utils=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libclippy_utils-ff2c2aaa2e33fd1f.rlib" "--extern" "serde_derive=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libserde_derive-f7c308c5f8e7b09c.so" "--extern" "derive_new=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps/libderive_new-a2f771a227dafeba.so" "--extern" "clippy_lints=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libclippy_lints-312563538883ae22.rlib" "--extern" "futures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libfutures-7a940bbcc1286cdc.rlib" "--extern" "rustc_semver=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/librustc_semver-963bbd3f89834643.rlib" "--extern" "parking_lot=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libparking_lot-3d962e7e23a10c27.rlib" "--extern" "quote=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libquote-e39b631048351f36.rlib" "--extern" "syn=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libsyn-7b5cc279bd04eca8.rlib" "--extern" "tokio=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libtokio-e0524b7e2611e851.rlib" "--extern" "regex=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libregex-619ac20e364f2b2c.rlib" "--extern" "serde=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libserde-edad3f22f7291185.rlib" "--edition=2021" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/test/ui/undropped_manually_drops.stage-id.aux"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
{"message":"call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes","code":{"code":"clippy::drop_non_drop","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/undropped_manually_drops.rs","byte_start":458,"byte_end":471,"line_start":15,"line_end":15,"column_start":5,"column_end":18,"is_primary":true,"text":[{"text":"    drop(manual1);","highlight_start":5,"highlight_end":18}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"argument has type `std::mem::ManuallyDrop<S>`","code":null,"level":"note","spans":[{"file_name":"tests/ui/undropped_manually_drops.rs","byte_start":463,"byte_end":470,"line_start":15,"line_end":15,"column_start":10,"column_end":17,"is_primary":true,"text":[{"text":"    drop(manual1);","highlight_start":10,"highlight_end":17}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":null},{"message":"`-D clippy::drop-non-drop` implied by `-D warnings`","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends its contained lifetimes\n  --> tests/ui/undropped_manually_drops.rs:15:5\n   |\nLL |     drop(manual1);\n   |     ^^^^^^^^^^^^^\n   |\nnote: argument has type `std::mem::ManuallyDrop<S>`\n  --> tests/ui/undropped_manually_drops.rs:15:10\n   |\nLL |     drop(manual1);\n   |          ^^^^^^^\n   = note: `-D clippy::drop-non-drop` implied by `-D warnings`\n\n"}

------------------------------------------

thread 'compile_test' panicked at 'Some tests failed', /cargo/registry/src/index.crates.io-6f17d22bba15001f/compiletest_rs-0.9.0/src/lib.rs:111:22

ssbr · 2023-02-10T19:55:00Z

I figured out why tests continue to pass locally but fail remotely. The failure message says:

failures:
    [ui] ui/undropped_manually_drops.rs

... however, there is no tests/ui/undropped_manually_drops.rs, and running ./x.py test or ./x.py test tests/ui does not catch this failure. Instead, the test is actually in src/tools/clippy/tests/ui/undropped_manually_drops.rs, so I need to run ./x.py test clippy or similar. I think. Right now, that causes an ICE:

thread 'rustc' panicked at 'no entry found for key', compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs:655:9

... so I'm going to wait until next week, and hopefully the ICE is fixed by then.

bors · 2023-02-26T02:11:14Z

☔ The latest upstream changes (presumably #108464) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2023-05-28T13:32:27Z

@ssbr
ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

JohnCSimon · 2023-08-27T06:13:05Z

@ssbr

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

rust-highfive assigned compiler-errors Sep 8, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 8, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2022

ssbr force-pushed the master branch from cdb6404 to b208892 Compare September 8, 2022 20:46

rust-highfive assigned estebank and unassigned compiler-errors Sep 23, 2022

estebank reviewed Nov 10, 2022

View reviewed changes

rustbot assigned lcnr and unassigned estebank Nov 10, 2022

lcnr reviewed Nov 11, 2022

View reviewed changes

ssbr force-pushed the master branch from 21c33ba to 49e18b4 Compare November 14, 2022 20:34

lcnr reviewed Nov 15, 2022

View reviewed changes

src/test/ui/manually_drop_attr/manually_drop-nodestructor.rs Outdated Show resolved Hide resolved

src/test/ui/manually_drop_attr/manually_drop-destructor.rs Outdated Show resolved Hide resolved

ssbr force-pushed the master branch from 19d09cf to 46b6233 Compare November 15, 2022 22:50

matthiaskrgr mentioned this pull request Nov 16, 2022

Rollup of 9 pull requests #104489

Closed

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 Nov 16, 2022

This comment has been minimized.

Sign in to view

flip1995 reviewed Dec 30, 2022

View reviewed changes

ssbr and others added 6 commits February 6, 2023 14:12

Replace manually_drop lang item with attribute.

fb064e3

Co-authored-by: lcnr <rust@lcnr.de>

Improve error message for #![manually_drop]

8090b31

Fix bad-item tests, which I apparently forgot to do

8b0f5f8

Add an enum variant per review comment

89f4a86

Fix test.

8a23f4c

... honestly, I'd thought I'd fixed this and tested before, this was my bad. All it took was `./x.py test src/test/ui --bless` :(

Ah. Move tests into tests dir, and --bless them.

8e01e2a

ssbr force-pushed the master branch from 36ac71d to 8e01e2a Compare February 6, 2023 22:29

This comment has been minimized.

Sign in to view

... actually run git add...

3468fac

JohnCSimon closed this Aug 27, 2023

rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Aug 27, 2023

Add the #[manually_drop] attribute. #101586

Add the #[manually_drop] attribute. #101586

Conversation

ssbr commented Sep 8, 2022 • edited Loading

The meaning of #[manually_drop]

Insignificant Drop

Users that expect adt.is_manually_drop() <-> "is std::mem::ManuallyDrop"

To-do in future PRs

rustbot commented Sep 8, 2022

rust-highfive commented Sep 8, 2022

programmerjake commented Sep 9, 2022

ssbr commented Sep 9, 2022

compiler-errors commented Sep 23, 2022

bors commented Sep 27, 2022

estebank commented Sep 27, 2022

ssbr commented Sep 27, 2022

estebank commented Nov 10, 2022

estebank Nov 10, 2022

Choose a reason for hiding this comment

ssbr Nov 11, 2022

Choose a reason for hiding this comment

estebank Nov 10, 2022

Choose a reason for hiding this comment

ssbr Nov 14, 2022

Choose a reason for hiding this comment

estebank commented Nov 10, 2022

lcnr left a comment

Choose a reason for hiding this comment

lcnr Nov 11, 2022

Choose a reason for hiding this comment

ssbr Nov 14, 2022

Choose a reason for hiding this comment

ssbr commented Nov 14, 2022

ssbr commented Nov 14, 2022

rustbot commented Nov 14, 2022

lcnr commented Nov 15, 2022

ssbr commented Nov 15, 2022

lcnr left a comment

Choose a reason for hiding this comment

ssbr commented Nov 15, 2022 • edited Loading

matthiaskrgr commented Nov 16, 2022

ssbr commented Nov 16, 2022

This comment has been minimized.

ssbr commented Dec 7, 2022

lcnr commented Dec 8, 2022

flip1995 Dec 30, 2022

Choose a reason for hiding this comment

Dylan-DPC commented Jan 25, 2023

ssbr commented Feb 6, 2023

This comment has been minimized.

rust-log-analyzer commented Feb 8, 2023

ssbr commented Feb 10, 2023

bors commented Feb 26, 2023

JohnCSimon commented May 28, 2023 • edited by lcnr Loading

JohnCSimon commented Aug 27, 2023

Add the `#[manually_drop]` attribute. #101586

Add the `#[manually_drop]` attribute. #101586

ssbr commented Sep 8, 2022 •

edited

Loading

The meaning of `#[manually_drop]`

Users that expect `adt.is_manually_drop()` <-> "is `std::mem::ManuallyDrop`"

ssbr commented Nov 15, 2022 •

edited

Loading

JohnCSimon commented May 28, 2023 •

edited by lcnr

Loading