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

Add lint for using a type with a destructor in a zero-length repeat expr #74857

Closed
Closed
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
35 changes: 33 additions & 2 deletions src/librustc_mir/borrow_check/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use either::Either;

use rustc_data_structures::frozen::Frozen;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::struct_span_err;
use rustc_errors::{struct_span_err, Applicability};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::lang_items::{CoerceUnsizedTraitLangItem, CopyTraitLangItem, SizedTraitLangItem};
Expand All @@ -30,6 +30,7 @@ use rustc_middle::ty::{
self, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, RegionVid, ToPredicate, Ty,
TyCtxt, UserType, UserTypeAnnotationIndex, WithConstness,
};
use rustc_session::lint::builtin::ZERO_REPEAT_WITH_DROP;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::VariantIdx;
use rustc_trait_selection::infer::InferCtxtExt as _;
Expand Down Expand Up @@ -1993,7 +1994,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// than 1.
// If the length is larger than 1, the repeat expression will need to copy the
// element, so we require the `Copy` trait.
if len.try_eval_usize(tcx, self.param_env).map_or(true, |len| len > 1) {
let len_val = len.try_eval_usize(tcx, self.param_env);
if len_val.map_or(true, |len| len > 1) {
if let Operand::Move(_) = operand {
// While this is located in `nll::typeck` this error is not an NLL error, it's
// a required check to make sure that repeated elements implement `Copy`.
Expand Down Expand Up @@ -2038,6 +2040,35 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
);
}
}
} else if let Some(0) = len_val {
let ty = operand.ty(body, tcx);
if ty.needs_drop(tcx, self.param_env) {
let source_info = body.source_info(location);
let lint_root = match &body.source_scopes[source_info.scope].local_data {
ClearCrossCrate::Set(data) => data.lint_root,
_ => tcx.hir().as_local_hir_id(self.mir_def_id),
};

tcx.struct_span_lint_hir(
ZERO_REPEAT_WITH_DROP,
lint_root,
source_info.span,
|lint| {
lint
.build("used a type with a destructor in a zero-length repeat expression")
.note(&format!("the value used here has type `{}`, which may have a destructor", ty))
Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally, we would use the proper spans for these notes, but it doesn't seem worth the effort to pass them through into the built MIR. We could move this to HIR typeck, but it seemed preferrable to keep this repeat-length checking logic in one place.

.note("a length of zero is used, which will cause the value to be dropped in a strange location")
.span_suggestion(
source_info.span,
"consider using an empty array expression",
"[]".to_string(),
Applicability::MaybeIncorrect
)
.emit();

}
);
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,12 @@ declare_lint! {
};
}

declare_lint! {
pub ZERO_REPEAT_WITH_DROP,
Warn,
"detects using a type with a destructor in an zero-length array repeat expression"
}

declare_lint_pass! {
/// Does nothing as a lint pass, but registers some `Lint`s
/// that are used by other parts of the compiler.
Expand Down Expand Up @@ -623,6 +629,7 @@ declare_lint_pass! {
UNSAFE_OP_IN_UNSAFE_FN,
INCOMPLETE_INCLUDE,
CENUM_IMPL_DROP_CAST,
ZERO_REPEAT_WITH_DROP,
]
}

Expand Down
36 changes: 36 additions & 0 deletions src/test/ui/lint/lint-zero-repeat-with-drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#![feature(const_generics)]
#![allow(incomplete_features)]
#![deny(zero_repeat_with_drop)]

const ZERO: usize = 1 * 0;

const fn make_val<T>(val: T) -> T { val }

struct NoDropOrCopy;
struct WithDropGlue(String);

fn foo<T, V: Copy, F: Fn() -> T, const N: usize>(not_copy: F, copy: V) {
// All of these should triger the lint
let _ = [not_copy(); 0]; //~ ERROR used a type
let _ = [not_copy(); 1 - 1]; //~ ERROR used a type
let _ = [not_copy(); ZERO]; //~ ERROR used a type
let _ = [Some(not_copy()); 0]; //~ ERROR used a type
let _ = [None::<T>; 0]; //~ ERROR used a type
let _ = [make_val(not_copy()); 0]; //~ ERROR used a type
let _ = [String::new(); 0]; //~ ERROR used a type
let _ = [WithDropGlue(String::new()); 0]; //~ ERROR used a type

// None of these should trigger the lint
let _ = [copy; 0];
let _ = [Some(copy); 0];
let _ = [NoDropOrCopy; 0];
let _ = [not_copy(); 1];
let _ = [copy; N];
}

fn allow_it() {
#[allow(zero_repeat_with_drop)]
let _ = [WithDropGlue(String::new()); 1 - 1];
}

fn main() {}
79 changes: 79 additions & 0 deletions src/test/ui/lint/lint-zero-repeat-with-drop.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:14:13
|
LL | let _ = [not_copy(); 0];
| ^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
note: the lint level is defined here
--> $DIR/lint-zero-repeat-with-drop.rs:3:9
|
LL | #![deny(zero_repeat_with_drop)]
| ^^^^^^^^^^^^^^^^^^^^^
= note: the value used here has type `T`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:15:13
|
LL | let _ = [not_copy(); 1 - 1];
| ^^^^^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `T`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:16:13
|
LL | let _ = [not_copy(); ZERO];
| ^^^^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `T`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:17:13
|
LL | let _ = [Some(not_copy()); 0];
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `std::option::Option<T>`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:18:13
|
LL | let _ = [None::<T>; 0];
| ^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `std::option::Option<T>`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:19:13
|
LL | let _ = [make_val(not_copy()); 0];
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `T`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:20:13
|
LL | let _ = [String::new(); 0];
| ^^^^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `std::string::String`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: used a type with a destructor in a zero-length repeat expression
--> $DIR/lint-zero-repeat-with-drop.rs:21:13
|
LL | let _ = [WithDropGlue(String::new()); 0];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an empty array expression: `[]`
|
= note: the value used here has type `WithDropGlue`, which may have a destructor
= note: a length of zero is used, which will cause the value to be dropped in a strange location

error: aborting due to 8 previous errors