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

Check that #[may_dangle] is properly applied #129235

Merged
merged 2 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,9 @@ passes_macro_export_on_decl_macro =
passes_macro_use =
`#[{$name}]` only has an effect on `extern crate` and modules

passes_may_dangle =
`#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl

passes_maybe_string_interpolation = you might have meant to use string interpolation in this string literal
passes_missing_const_err =
attributes `#[rustc_const_unstable]` and `#[rustc_const_stable]` require the function or method to be `const`
Expand Down Expand Up @@ -475,8 +478,8 @@ passes_multiple_start_functions =
.previous = previous `#[start]` function here

passes_must_not_suspend =
`must_not_suspend` attribute should be applied to a struct, enum, or trait
.label = is not a struct, enum, or trait
`must_not_suspend` attribute should be applied to a struct, enum, union, or trait
.label = is not a struct, enum, union, or trait

passes_must_use_async =
`must_use` attribute on `async` functions applies to the anonymous `Future` returned by the function, not the value within
Expand Down
25 changes: 23 additions & 2 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
[sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
[sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
[sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
[sym::may_dangle, ..] => self.check_may_dangle(hir_id, attr),
[sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
[sym::rustc_allow_incoherent_impl, ..] => {
self.check_allow_incoherent_impl(attr, span, target)
Expand Down Expand Up @@ -255,7 +256,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
| sym::cfg_attr
// need to be fixed
| sym::cfi_encoding // FIXME(cfi_encoding)
| sym::may_dangle // FIXME(dropck_eyepatch)
| sym::pointee // FIXME(derive_smart_pointer)
| sym::omit_gdb_pretty_printer_section // FIXME(omit_gdb_pretty_printer_section)
| sym::used // handled elsewhere to restrict to static items
Expand Down Expand Up @@ -1363,7 +1363,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Checks if `#[must_not_suspend]` is applied to a function.
/// Checks if `#[must_not_suspend]` is applied to a struct, enum, union, or trait.
fn check_must_not_suspend(&self, attr: &Attribute, span: Span, target: Target) {
match target {
Target::Struct | Target::Enum | Target::Union | Target::Trait => {}
Expand All @@ -1373,6 +1373,27 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Checks if `#[may_dangle]` is applied to a lifetime or type generic parameter in `Drop` impl.
fn check_may_dangle(&self, hir_id: HirId, attr: &Attribute) {
if let hir::Node::GenericParam(param) = self.tcx.hir_node(hir_id)
&& matches!(
param.kind,
hir::GenericParamKind::Lifetime { .. } | hir::GenericParamKind::Type { .. }
)
&& matches!(param.source, hir::GenericParamSource::Generics)
&& let parent_hir_id = self.tcx.parent_hir_id(hir_id)
&& let hir::Node::Item(item) = self.tcx.hir_node(parent_hir_id)
&& let hir::ItemKind::Impl(impl_) = item.kind
&& let Some(trait_) = impl_.of_trait
Comment on lines +1385 to +1387
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&& let hir::Node::Item(item) = self.tcx.hir_node(parent_hir_id)
&& let hir::ItemKind::Impl(impl_) = item.kind
&& let Some(trait_) = impl_.of_trait
&& let hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(impl_), .. }) = self.tcx.hir_node(parent_hir_id)
&& let Some(trait_) = impl_.of_trait

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This leads to somewhat ugly formatting:

&& let hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(impl_), .. }) =
    self.tcx.hir_node(parent_hir_id)

I think the current version is nicer, but I don’t feel that strong about it, so I can make this change if you prefer it.

&& let Some(def_id) = trait_.trait_def_id()
&& self.tcx.is_lang_item(def_id, hir::LangItem::Drop)
{
return;
}

self.dcx().emit_err(errors::InvalidMayDangle { attr_span: attr.span });
}

/// Checks if `#[cold]` is applied to a non-function.
fn check_cold(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) {
match target {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,13 @@ pub struct NonExportedMacroInvalidAttrs {
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_may_dangle)]
pub struct InvalidMayDangle {
#[primary_span]
pub attr_span: Span,
}

#[derive(LintDiagnostic)]
#[diag(passes_unused_duplicate)]
pub struct UnusedDuplicate {
Expand Down
53 changes: 53 additions & 0 deletions tests/ui/attributes/may_dangle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#![feature(dropck_eyepatch)]

struct Implee1<'a, T, const N: usize>(&'a T);
struct Implee2<'a, T, const N: usize>(&'a T);
struct Implee3<'a, T, const N: usize>(&'a T);
trait NotDrop {}

unsafe impl<#[may_dangle] 'a, T, const N: usize> NotDrop for Implee1<'a, T, N> {}
//~^ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl

unsafe impl<'a, #[may_dangle] T, const N: usize> NotDrop for Implee2<'a, T, N> {}
//~^ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl

unsafe impl<'a, T, #[may_dangle] const N: usize> Drop for Implee1<'a, T, N> {
//~^ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl
fn drop(&mut self) {}
}

// Ok, lifetime param in a `Drop` impl.
unsafe impl<#[may_dangle] 'a, T, const N: usize> Drop for Implee2<'a, T, N> {
fn drop(&mut self) {}
}

// Ok, type param in a `Drop` impl.
unsafe impl<'a, #[may_dangle] T, const N: usize> Drop for Implee3<'a, T, N> {
fn drop(&mut self) {}
}

// Check that this check is not textual.
mod fake {
trait Drop {
fn drop(&mut self);
}
struct Implee<T>(T);

unsafe impl<#[may_dangle] T> Drop for Implee<T> {
//~^ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl
fn drop(&mut self) {}
}
}

#[may_dangle] //~ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl
struct Dangling;

#[may_dangle] //~ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl
impl NotDrop for () {
}

#[may_dangle] //~ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl
fn main() {
#[may_dangle] //~ ERROR must be applied to a lifetime or type generic parameter in `Drop` impl
let () = ();
}
50 changes: 50 additions & 0 deletions tests/ui/attributes/may_dangle.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:8:13
|
LL | unsafe impl<#[may_dangle] 'a, T, const N: usize> NotDrop for Implee1<'a, T, N> {}
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:11:17
|
LL | unsafe impl<'a, #[may_dangle] T, const N: usize> NotDrop for Implee2<'a, T, N> {}
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:14:20
|
LL | unsafe impl<'a, T, #[may_dangle] const N: usize> Drop for Implee1<'a, T, N> {
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:42:1
|
LL | #[may_dangle]
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:45:1
|
LL | #[may_dangle]
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:49:1
|
LL | #[may_dangle]
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:51:5
|
LL | #[may_dangle]
| ^^^^^^^^^^^^^

error: `#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
--> $DIR/may_dangle.rs:36:17
|
LL | unsafe impl<#[may_dangle] T> Drop for Implee<T> {
| ^^^^^^^^^^^^^

error: aborting due to 8 previous errors

4 changes: 2 additions & 2 deletions tests/ui/lint/must_not_suspend/other_items.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: `must_not_suspend` attribute should be applied to a struct, enum, or trait
error: `must_not_suspend` attribute should be applied to a struct, enum, union, or trait
--> $DIR/other_items.rs:5:1
|
LL | #[must_not_suspend]
| ^^^^^^^^^^^^^^^^^^^
LL | mod inner {}
| ------------ is not a struct, enum, or trait
| ------------ is not a struct, enum, union, or trait

error: aborting due to 1 previous error

4 changes: 2 additions & 2 deletions tests/ui/lint/must_not_suspend/return.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error: `must_not_suspend` attribute should be applied to a struct, enum, or trait
error: `must_not_suspend` attribute should be applied to a struct, enum, union, or trait
--> $DIR/return.rs:5:1
|
LL | #[must_not_suspend]
| ^^^^^^^^^^^^^^^^^^^
LL | / fn foo() -> i32 {
LL | | 0
LL | | }
| |_- is not a struct, enum, or trait
| |_- is not a struct, enum, union, or trait

error: aborting due to 1 previous error

Loading