must_not_suspend impl

This commit is contained in:
Gus Wynn 2021-09-04 19:36:51 -07:00
parent 5d2a410ff7
commit 2271376fb2
19 changed files with 587 additions and 5 deletions

View File

@ -679,6 +679,10 @@ declare_features! (
/// Allows `let...else` statements.
(active, let_else, "1.56.0", Some(87335), None),
/// Allows `#[must_not_suspend]`.
(active, must_not_suspend, "1.56.0", Some(83310), None),
// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------

View File

@ -202,6 +202,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
ungated!(forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#)),
ungated!(deny, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#)),
ungated!(must_use, Normal, template!(Word, NameValueStr: "reason")),
gated!(
must_not_suspend, Normal, template!(Word, NameValueStr: "reason"), must_not_suspend,
experimental!(must_not_suspend)
),
// FIXME(#14407)
ungated!(
deprecated, Normal,

View File

@ -298,6 +298,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
UNUSED_LABELS,
UNUSED_PARENS,
UNUSED_BRACES,
MUST_NOT_SUSPEND,
REDUNDANT_SEMICOLONS
);

View File

@ -314,6 +314,13 @@ declare_lint! {
"imports that are never used"
}
declare_lint! {
/// [the reference]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
pub MUST_NOT_SUSPEND,
Warn,
"Use of a `#[must_not_suspend]` value across a yield point",
}
declare_lint! {
/// The `unused_extern_crates` lint guards against `extern crate` items
/// that are never used.
@ -2993,6 +3000,7 @@ declare_lint_pass! {
CENUM_IMPL_DROP_CAST,
CONST_EVALUATABLE_UNCHECKED,
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
MUST_NOT_SUSPEND,
UNINHABITED_STATIC,
FUNCTION_ITEM_REFERENCES,
USELESS_DEPRECATED,

View File

@ -104,6 +104,7 @@ impl CheckAttrVisitor<'tcx> {
sym::default_method_body_is_const => {
self.check_default_method_body_is_const(attr, span, target)
}
sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target),
sym::rustc_const_unstable
| sym::rustc_const_stable
| sym::unstable
@ -1014,6 +1015,21 @@ impl CheckAttrVisitor<'tcx> {
is_valid
}
/// Checks if `#[must_not_suspend]` is applied to a function. Returns `true` if valid.
fn check_must_not_suspend(&self, attr: &Attribute, span: &Span, target: Target) -> bool {
match target {
Target::Fn | Target::Method(..) | Target::ForeignFn | Target::Closure => {
self.tcx
.sess
.struct_span_err(attr.span, "`must_not_suspend` attribute should be applied to a struct, enum, `impl Trait`, or `dyn Trait`")
.span_label(*span, "is a function")
.emit();
false
}
_ => true,
}
}
/// Checks if `#[cold]` is applied to a non-function. Returns `true` if valid.
fn check_cold(&self, hir_id: HirId, attr: &Attribute, span: &Span, target: Target) {
match target {

View File

@ -836,6 +836,7 @@ symbols! {
mul,
mul_assign,
mul_with_overflow,
must_not_suspend,
must_use,
mut_ptr,
mut_slice_ptr,

View File

@ -5,6 +5,7 @@
use super::FnCtxt;
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_errors::pluralize;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::DefId;
@ -12,9 +13,11 @@ use rustc_hir::hir_id::HirIdSet;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind};
use rustc_middle::middle::region::{self, YieldData};
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::symbol::sym;
use rustc_span::Span;
use smallvec::SmallVec;
use tracing::debug;
struct InteriorVisitor<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
@ -36,6 +39,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
fn record(
&mut self,
ty: Ty<'tcx>,
hir_id: HirId,
scope: Option<region::Scope>,
expr: Option<&'tcx Expr<'tcx>>,
source_span: Span,
@ -117,6 +121,20 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
} else {
// Insert the type into the ordered set.
let scope_span = scope.map(|s| s.span(self.fcx.tcx, self.region_scope_tree));
check_must_not_suspend_ty(
self.fcx,
ty::ParamEnv::empty(),
ty,
hir_id,
expr,
source_span,
yield_data.span,
"",
"",
1,
);
self.types.insert(ty::GeneratorInteriorTypeCause {
span: source_span,
ty: &ty,
@ -290,7 +308,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
if let PatKind::Binding(..) = pat.kind {
let scope = self.region_scope_tree.var_scope(pat.hir_id.local_id);
let ty = self.fcx.typeck_results.borrow().pat_ty(pat);
self.record(ty, Some(scope), None, pat.span, false);
self.record(ty, pat.hir_id, Some(scope), None, pat.span, false);
}
}
@ -342,7 +360,14 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
// If there are adjustments, then record the final type --
// this is the actual value that is being produced.
if let Some(adjusted_ty) = self.fcx.typeck_results.borrow().expr_ty_adjusted_opt(expr) {
self.record(adjusted_ty, scope, Some(expr), expr.span, guard_borrowing_from_pattern);
self.record(
adjusted_ty,
expr.hir_id,
scope,
Some(expr),
expr.span,
guard_borrowing_from_pattern,
);
}
// Also record the unadjusted type (which is the only type if
@ -380,9 +405,23 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
tcx.mk_region(ty::RegionKind::ReErased),
ty::TypeAndMut { ty, mutbl: hir::Mutability::Not },
);
self.record(ref_ty, scope, Some(expr), expr.span, guard_borrowing_from_pattern);
self.record(
ref_ty,
expr.hir_id,
scope,
Some(expr),
expr.span,
guard_borrowing_from_pattern,
);
}
self.record(ty, scope, Some(expr), expr.span, guard_borrowing_from_pattern);
self.record(
ty,
expr.hir_id,
scope,
Some(expr),
expr.span,
guard_borrowing_from_pattern,
);
} else {
self.fcx.tcx.sess.delay_span_bug(expr.span, "no type for node");
}
@ -409,3 +448,263 @@ impl<'a, 'tcx> Visitor<'tcx> for ArmPatCollector<'a> {
}
}
}
// Returns whether it emitted a diagnostic or not
// Note that this fn and the proceding one are based on the code
// for creating must_use diagnostics
pub fn check_must_not_suspend_ty<'tcx>(
fcx: &FnCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
hir_id: HirId,
expr: Option<&'tcx Expr<'tcx>>,
source_span: Span,
yield_span: Span,
descr_pre: &str,
descr_post: &str,
plural_len: usize,
) -> bool {
if ty.is_unit()
|| fcx.tcx.is_ty_uninhabited_from(fcx.tcx.parent_module(hir_id).to_def_id(), ty, param_env)
{
return true;
}
let plural_suffix = pluralize!(plural_len);
let emitted = match *ty.kind() {
ty::Adt(..) if ty.is_box() => {
let boxed_ty = ty.boxed_ty();
let descr_pre = &format!("{}boxed ", descr_pre);
check_must_not_suspend_ty(
fcx,
param_env,
boxed_ty,
hir_id,
expr,
source_span,
yield_span,
descr_pre,
descr_post,
plural_len,
)
}
ty::Adt(def, _) => check_must_not_suspend_def(
fcx.tcx,
def.did,
hir_id,
source_span,
yield_span,
descr_pre,
descr_post,
),
ty::Opaque(def, _) => {
let mut has_emitted = false;
for &(predicate, _) in fcx.tcx.explicit_item_bounds(def) {
// We only look at the `DefId`, so it is safe to skip the binder here.
if let ty::PredicateKind::Trait(ref poly_trait_predicate) =
predicate.kind().skip_binder()
{
let def_id = poly_trait_predicate.trait_ref.def_id;
let descr_pre = &format!("{}implementer{} of ", descr_pre, plural_suffix,);
if check_must_not_suspend_def(
fcx.tcx,
def_id,
hir_id,
source_span,
yield_span,
descr_pre,
descr_post,
) {
has_emitted = true;
break;
}
}
}
has_emitted
}
ty::Dynamic(binder, _) => {
let mut has_emitted = false;
for predicate in binder.iter() {
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() {
let def_id = trait_ref.def_id;
let descr_post = &format!(" trait object{}{}", plural_suffix, descr_post,);
if check_must_not_suspend_def(
fcx.tcx,
def_id,
hir_id,
source_span,
yield_span,
descr_pre,
descr_post,
) {
has_emitted = true;
break;
}
}
}
has_emitted
}
ty::Tuple(ref tys) => {
let mut has_emitted = false;
/*
let spans = if let hir::ExprKind::Tup(comps) = &expr.kind {
debug_assert_eq!(comps.len(), tys.len());
comps.iter().map(|e| e.span).collect()
} else {
vec![]
};
*/
let spans = vec![];
for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() {
let descr_post = &format!(" in tuple element {}", i);
let span = *spans.get(i).unwrap_or(&source_span);
if check_must_not_suspend_ty(
fcx, param_env, ty, hir_id, expr, span, yield_span, descr_pre, descr_post,
plural_len,
) {
has_emitted = true;
}
}
has_emitted
}
ty::Array(ty, len) => match len.try_eval_usize(fcx.tcx, param_env) {
// If the array is empty we don't lint, to avoid false positives
Some(0) | None => false,
// If the array is definitely non-empty, we can do `#[must_use]` checking.
Some(n) => {
let descr_pre = &format!("{}array{} of ", descr_pre, plural_suffix,);
check_must_not_suspend_ty(
fcx,
param_env,
ty,
hir_id,
expr,
source_span,
yield_span,
descr_pre,
descr_post,
n as usize + 1,
)
}
},
_ => false,
};
// Don't move onto the "return value" path if we already sent a diagnostic
if emitted {
return true;
}
match expr {
Some(expr) => match expr.kind {
hir::ExprKind::Call(ref callee, _) => {
match callee.kind {
hir::ExprKind::Path(ref qpath) => {
match fcx.typeck_results.borrow().qpath_res(qpath, callee.hir_id) {
Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) => {
check_must_not_suspend_def(
fcx.tcx,
def_id,
hir_id,
source_span,
yield_span,
"return value of ",
"",
)
}
// `Res::Local` if it was a closure, for which we
// do not currently support must-not-suspend linting
_ => false,
}
}
_ => false,
}
}
hir::ExprKind::MethodCall(..) => {
if let Some(def_id) = fcx.typeck_results.borrow().type_dependent_def_id(expr.hir_id)
{
check_must_not_suspend_def(
fcx.tcx,
def_id,
hir_id,
source_span,
yield_span,
"return value of ",
"",
)
} else {
false
}
}
_ => false,
},
None => false,
}
}
fn check_must_not_suspend_def(
tcx: TyCtxt<'_>,
def_id: DefId,
hir_id: HirId,
source_span: Span,
yield_span: Span,
descr_pre_path: &str,
descr_post_path: &str,
) -> bool {
for attr in tcx.get_attrs(def_id).iter() {
if attr.has_name(sym::must_not_suspend) {
tcx.struct_span_lint_hir(
rustc_session::lint::builtin::MUST_NOT_SUSPEND,
hir_id,
source_span,
|lint| {
let msg = format!(
"{}`{}`{} held across a yield point, but should not be",
descr_pre_path,
tcx.def_path_str(def_id),
descr_post_path
);
let mut err = lint.build(&msg);
// Add optional reason note
if let Some(note) = attr.value_str() {
err.note(&note.as_str());
}
// add span pointing to the offending yield/await)
err.span_label(yield_span, "The value is held across this yield point");
// Add some quick suggestions on what to do
err.span_help(
source_span,
"`drop` this value before the yield point, or use a block (`{ ... }`) \"
to shrink its scope",
);
err.emit();
},
);
/*
cx.struct_span_lint(UNUSED_MUST_USE, span, |lint| {
let msg = format!(
"unused {}`{}`{} that must be used",
descr_pre_path,
cx.tcx.def_path_str(def_id),
descr_post_path
);
let mut err = lint.build(&msg);
// check for #[must_use = "..."]
if let Some(note) = attr.value_str() {
err.note(&note.as_str());
}
err.emit();
});
*/
return true;
}
}
false
}

View File

@ -0,0 +1,25 @@
// edition:2018
#![feature(must_not_suspend)]
#![deny(must_not_suspend)]
#[must_not_suspend = "You gotta use Umm's, ya know?"]
struct Umm {
i: i64
}
fn bar() -> Box<Umm> {
Box::new(Umm {
i: 1
})
}
async fn other() {}
pub async fn uhoh() {
let _guard = bar(); //~ boxed `Umm` held across
other().await;
}
fn main() {
}

View File

@ -0,0 +1,23 @@
error: boxed `Umm` held across a yield point, but should not be
--> $DIR/boxed.rs:20:9
|
LL | let _guard = bar();
| ^^^^^^
LL | other().await;
| ------------- The value is held across this yield point
|
note: the lint level is defined here
--> $DIR/boxed.rs:3:9
|
LL | #![deny(must_not_suspend)]
| ^^^^^^^^^^^^^^^^
= note: You gotta use Umm's, ya know?
help: `drop` this value before the yield point, or use a block (`{ ... }`) "
to shrink its scope
--> $DIR/boxed.rs:20:9
|
LL | let _guard = bar();
| ^^^^^^
error: aborting due to previous error

View File

@ -0,0 +1,9 @@
// edition:2018
#[must_not_suspend = "You gotta use Umm's, ya know?"] //~ the `#[must_not_suspend]`
struct Umm {
_i: i64
}
fn main() {
}

View File

@ -0,0 +1,12 @@
error[E0658]: the `#[must_not_suspend]` attribute is an experimental feature
--> $DIR/feature-gate-must_not_suspend.rs:3:1
|
LL | #[must_not_suspend = "You gotta use Umm's, ya know?"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #83310 <https://github.com/rust-lang/rust/issues/83310> for more information
= help: add `#![feature(must_not_suspend)]` to the crate attributes to enable
error: aborting due to previous error
For more information about this error, try `rustc --explain E0658`.

View File

@ -0,0 +1,9 @@
// edition:2018
#![feature(must_not_suspend)]
#![deny(must_not_suspend)]
#[must_not_suspend] //~ attribute should be
fn foo() -> i32 {
0
}
fn main() {}

View File

@ -0,0 +1,12 @@
error: `must_not_suspend` attribute should be applied to a struct, enum, `impl Trait`, or `dyn Trait`
--> $DIR/return.rs:5:1
|
LL | #[must_not_suspend]
| ^^^^^^^^^^^^^^^^^^^
LL | / fn foo() -> i32 {
LL | | 0
LL | | }
| |_- is a function
error: aborting due to previous error

View File

@ -0,0 +1,28 @@
// edition:2018
#![feature(must_not_suspend)]
#![deny(must_not_suspend)]
#[must_not_suspend]
trait Wow {}
impl Wow for i32 {}
fn r#impl() -> impl Wow {
1
}
fn r#dyn() -> Box<dyn Wow> {
Box::new(1)
}
async fn other() {}
pub async fn uhoh() {
let _guard1 = r#impl(); //~ implementer of `Wow` held across
let _guard2 = r#dyn(); //~ boxed `Wow` trait object held across
other().await;
}
fn main() {
}

View File

@ -0,0 +1,39 @@
error: implementer of `Wow` held across a yield point, but should not be
--> $DIR/trait.rs:21:9
|
LL | let _guard1 = r#impl();
| ^^^^^^^
...
LL | other().await;
| ------------- The value is held across this yield point
|
note: the lint level is defined here
--> $DIR/trait.rs:3:9
|
LL | #![deny(must_not_suspend)]
| ^^^^^^^^^^^^^^^^
help: `drop` this value before the yield point, or use a block (`{ ... }`) "
to shrink its scope
--> $DIR/trait.rs:21:9
|
LL | let _guard1 = r#impl();
| ^^^^^^^
error: boxed `Wow` trait object held across a yield point, but should not be
--> $DIR/trait.rs:22:9
|
LL | let _guard2 = r#dyn();
| ^^^^^^^
LL |
LL | other().await;
| ------------- The value is held across this yield point
|
help: `drop` this value before the yield point, or use a block (`{ ... }`) "
to shrink its scope
--> $DIR/trait.rs:22:9
|
LL | let _guard2 = r#dyn();
| ^^^^^^^
error: aborting due to 2 previous errors

View File

@ -0,0 +1,25 @@
// edition:2018
#![feature(must_not_suspend)]
#![deny(must_not_suspend)]
#[must_not_suspend = "You gotta use Umm's, ya know?"]
struct Umm {
i: i64
}
fn bar() -> Umm {
Umm {
i: 1
}
}
async fn other() {}
pub async fn uhoh() {
let _guard = bar(); //~ `Umm` held across
other().await;
}
fn main() {
}

View File

@ -0,0 +1,23 @@
error: `Umm` held across a yield point, but should not be
--> $DIR/unit.rs:20:9
|
LL | let _guard = bar();
| ^^^^^^
LL | other().await;
| ------------- The value is held across this yield point
|
note: the lint level is defined here
--> $DIR/unit.rs:3:9
|
LL | #![deny(must_not_suspend)]
| ^^^^^^^^^^^^^^^^
= note: You gotta use Umm's, ya know?
help: `drop` this value before the yield point, or use a block (`{ ... }`) "
to shrink its scope
--> $DIR/unit.rs:20:9
|
LL | let _guard = bar();
| ^^^^^^
error: aborting due to previous error

View File

@ -0,0 +1,25 @@
// edition:2018
// run-pass
#![feature(must_not_suspend)]
#[must_not_suspend = "You gotta use Umm's, ya know?"]
struct Umm {
_i: i64
}
fn bar() -> Umm {
Umm {
_i: 1
}
}
async fn other() {}
pub async fn uhoh() {
let _guard = bar(); //~ `Umm` held across
other().await;
}
fn main() {
}

View File

@ -0,0 +1,19 @@
warning: `Umm` held across a yield point, but should not be
--> $DIR/warn.rs:20:9
|
LL | let _guard = bar();
| ^^^^^^
LL | other().await;
| ------------- The value is held across this yield point
|
= note: `#[warn(must_not_suspend)]` on by default
= note: You gotta use Umm's, ya know?
help: `drop` this value before the yield point, or use a block (`{ ... }`) "
to shrink its scope
--> $DIR/warn.rs:20:9
|
LL | let _guard = bar();
| ^^^^^^
warning: 1 warning emitted