Skip to content

AsyncDrop trait without sync Drop generates an error #142606

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+49 −1
Open

AsyncDrop trait without sync Drop generates an error #142606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the {$

hir_analysis_associated_type_trait_uninferred_generic_params_multipart_suggestion = use a fully qualified path with explicit lifetimes

hir_analysis_async_drop_without_sync_drop = `AsyncDrop` impl without `Drop` impl
.help = type implementing `AsyncDrop` trait must also implement `Drop` trait to be used in sync context and unwinds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which "sync contexts" are currently remaining besides unwinds?
Ideally async drop should just always be called, but while it's not it would be good to list the unimplemented cases somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sync context - any sync function, where AsyncDrop type is dropped inside.
fn foo() { _ = Dropee::new(); }, where Dropee has only AsyncDrop trait without Drop trait.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I thought cases like this are already reported as errors (they should be).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea that's an alternative to this PR (or that allows reverting this PR): error at use sites of sync drop of types that have no Drop impl but an AsyncDrop impl. This has some fun implications though, as we'd need to make sure that Copy and AsyncDrop also need to be mutually exclusive, just like Copy and Drop are. Or more generally: any logic checking for Drop needs to also check for AsyncDrop and handle that.

So it may be better long term to merge various things like adt_destructor and adt_async_destructor to only return a single Option with an enum inside for either an async or a sync variant, making it easier not to forget to check both together. There are probably many other functions that need similar treatment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, error at use sites of sync drop may help only after implementing async-drop support for unwinds. Otherwise, we still need both traits.


hir_analysis_auto_deref_reached_recursion_limit = reached the recursion limit while auto-dereferencing `{$ty}`
.label = deref recursion limit reached
.help = consider increasing the recursion limit by adding a `#![recursion_limit = "{$suggested_limit}"]` attribute to your crate (`{$crate_name}`)
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,15 @@ pub fn provide(providers: &mut Providers) {
}

fn adt_destructor(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::Destructor> {
tcx.calculate_dtor(def_id, always_applicable::check_drop_impl)
let dtor = tcx.calculate_dtor(def_id, always_applicable::check_drop_impl);
if dtor.is_none() && tcx.features().async_drop() {
if let Some(async_dtor) = adt_async_destructor(tcx, def_id) {
// When type has AsyncDrop impl, but doesn't have Drop impl, generate error
let span = tcx.def_span(async_dtor.impl_did);
tcx.dcx().emit_err(errors::AsyncDropWithoutSyncDrop { span });
}
}
dtor
}

fn adt_async_destructor(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::AsyncDestructor> {
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1712,3 +1712,11 @@ pub(crate) struct AbiCustomClothedFunction {
)]
pub naked_span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_async_drop_without_sync_drop)]
#[help]
pub(crate) struct AsyncDropWithoutSyncDrop {
#[primary_span]
pub span: Span,
}
19 changes: 19 additions & 0 deletions tests/ui/async-await/async-drop/async-without-sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//@ edition: 2024
#![feature(async_drop)]
#![allow(incomplete_features)]
#![crate_type = "lib"]

use std::future::AsyncDrop;
use std::pin::Pin;

async fn foo() {
let _st = St;
}

struct St;

impl AsyncDrop for St { //~ ERROR: `AsyncDrop` impl without `Drop` impl
async fn drop(self: Pin<&mut Self>) {
println!("123");
}
}
10 changes: 10 additions & 0 deletions tests/ui/async-await/async-drop/async-without-sync.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: `AsyncDrop` impl without `Drop` impl
--> $DIR/async-without-sync.rs:15:1
|
LL | impl AsyncDrop for St {
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: type implementing `AsyncDrop` trait must also implement `Drop` trait to be used in sync context and unwinds

error: aborting due to 1 previous error

Loading