]> git.lizzy.rs Git - rust.git/blobdiff - compiler/rustc_typeck/src/check/generator_interior.rs
Auto merge of #88865 - guswynn:must_not_suspend, r=oli-obk
[rust.git] / compiler / rustc_typeck / src / check / generator_interior.rs
index 3da9fd159a728e7e909e89980ee476041050eef1..2910ce6de689965c4adcde2668aedab785443790 100644 (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;
 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>,
@@ -30,12 +33,14 @@ struct InteriorVisitor<'a, 'tcx> {
     /// that they may succeed the said yield point in the post-order.
     guard_bindings: SmallVec<[SmallVec<[HirId; 4]>; 1]>,
     guard_bindings_set: HirIdSet,
+    linted_values: HirIdSet,
 }
 
 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 +122,23 @@ fn record(
             } else {
                 // Insert the type into the ordered set.
                 let scope_span = scope.map(|s| s.span(self.fcx.tcx, self.region_scope_tree));
+
+                if !self.linted_values.contains(&hir_id) {
+                    check_must_not_suspend_ty(
+                        self.fcx,
+                        ty,
+                        hir_id,
+                        SuspendCheckData {
+                            expr,
+                            source_span,
+                            yield_span: yield_data.span,
+                            plural_len: 1,
+                            ..Default::default()
+                        },
+                    );
+                    self.linted_values.insert(hir_id);
+                }
+
                 self.types.insert(ty::GeneratorInteriorTypeCause {
                     span: source_span,
                     ty: &ty,
@@ -163,6 +185,7 @@ pub fn resolve_interior<'a, 'tcx>(
         prev_unresolved_span: None,
         guard_bindings: <_>::default(),
         guard_bindings_set: <_>::default(),
+        linted_values: <_>::default(),
     };
     intravisit::walk_body(&mut visitor, body);
 
@@ -290,7 +313,7 @@ fn visit_pat(&mut self, pat: &'tcx Pat<'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 +365,14 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'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 +410,23 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'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 +453,173 @@ fn visit_pat(&mut self, pat: &'tcx Pat<'tcx>) {
         }
     }
 }
+
+#[derive(Default)]
+pub struct SuspendCheckData<'a, 'tcx> {
+    expr: Option<&'tcx Expr<'tcx>>,
+    source_span: Span,
+    yield_span: Span,
+    descr_pre: &'a str,
+    descr_post: &'a str,
+    plural_len: usize,
+}
+
+// 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
+//
+// Note that this technique was chosen over things like a `Suspend` marker trait
+// as it is simpler and has precendent in the compiler
+pub fn check_must_not_suspend_ty<'tcx>(
+    fcx: &FnCtxt<'_, 'tcx>,
+    ty: Ty<'tcx>,
+    hir_id: HirId,
+    data: SuspendCheckData<'_, 'tcx>,
+) -> bool {
+    if ty.is_unit()
+    // FIXME: should this check `is_ty_uninhabited_from`. This query is not available in this stage
+    // of typeck (before ReVar and RePlaceholder are removed), but may remove noise, like in
+    // `must_use`
+    // || fcx.tcx.is_ty_uninhabited_from(fcx.tcx.parent_module(hir_id).to_def_id(), ty, fcx.param_env)
+    {
+        return false;
+    }
+
+    let plural_suffix = pluralize!(data.plural_len);
+
+    match *ty.kind() {
+        ty::Adt(..) if ty.is_box() => {
+            let boxed_ty = ty.boxed_ty();
+            let descr_pre = &format!("{}boxed ", data.descr_pre);
+            check_must_not_suspend_ty(fcx, boxed_ty, hir_id, SuspendCheckData { descr_pre, ..data })
+        }
+        ty::Adt(def, _) => check_must_not_suspend_def(fcx.tcx, def.did, hir_id, data),
+        // FIXME: support adding the attribute to TAITs
+        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 ", data.descr_pre, plural_suffix);
+                    if check_must_not_suspend_def(
+                        fcx.tcx,
+                        def_id,
+                        hir_id,
+                        SuspendCheckData { descr_pre, ..data },
+                    ) {
+                        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, data.descr_post);
+                    if check_must_not_suspend_def(
+                        fcx.tcx,
+                        def_id,
+                        hir_id,
+                        SuspendCheckData { descr_post, ..data },
+                    ) {
+                        has_emitted = true;
+                        break;
+                    }
+                }
+            }
+            has_emitted
+        }
+        ty::Tuple(ref tys) => {
+            let mut has_emitted = false;
+            let spans = if let Some(hir::ExprKind::Tup(comps)) = data.expr.map(|e| &e.kind) {
+                debug_assert_eq!(comps.len(), tys.len());
+                comps.iter().map(|e| e.span).collect()
+            } else {
+                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(&data.source_span);
+                if check_must_not_suspend_ty(
+                    fcx,
+                    ty,
+                    hir_id,
+                    SuspendCheckData { descr_post, source_span: span, ..data },
+                ) {
+                    has_emitted = true;
+                }
+            }
+            has_emitted
+        }
+        ty::Array(ty, len) => {
+            let descr_pre = &format!("{}array{} of ", data.descr_pre, plural_suffix);
+            check_must_not_suspend_ty(
+                fcx,
+                ty,
+                hir_id,
+                SuspendCheckData {
+                    descr_pre,
+                    plural_len: len.try_eval_usize(fcx.tcx, fcx.param_env).unwrap_or(0) as usize
+                        + 1,
+                    ..data
+                },
+            )
+        }
+        _ => false,
+    }
+}
+
+fn check_must_not_suspend_def(
+    tcx: TyCtxt<'_>,
+    def_id: DefId,
+    hir_id: HirId,
+    data: SuspendCheckData<'_, '_>,
+) -> 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,
+                data.source_span,
+                |lint| {
+                    let msg = format!(
+                        "{}`{}`{} held across a suspend point, but should not be",
+                        data.descr_pre,
+                        tcx.def_path_str(def_id),
+                        data.descr_post,
+                    );
+                    let mut err = lint.build(&msg);
+
+                    // add span pointing to the offending yield/await
+                    err.span_label(data.yield_span, "the value is held across this suspend point");
+
+                    // Add optional reason note
+                    if let Some(note) = attr.value_str() {
+                        // FIXME(guswynn): consider formatting this better
+                        err.span_note(data.source_span, &note.as_str());
+                    }
+
+                    // Add some quick suggestions on what to do
+                    // FIXME: can `drop` work as a suggestion here as well?
+                    err.span_help(
+                        data.source_span,
+                        "consider using a block (`{ ... }`) \
+                        to shrink the value's scope, ending before the suspend point",
+                    );
+
+                    err.emit();
+                },
+            );
+
+            return true;
+        }
+    }
+    false
+}