From e8910440a24ecb2ddb08b4dbe170488ddf61fc87 Mon Sep 17 00:00:00 2001 From: liudingming Date: Mon, 9 Aug 2021 20:25:57 +0800 Subject: [PATCH 1/6] select obligations after `check_casts` Otherwise, we can get into a situation where you have a subtype obligation `#1 <: #2` pending, #1 is constrained by `check_casts`, but #2` is unaffected. Co-authored-by: Niko Matsakis --- compiler/rustc_typeck/src/check/fallback.rs | 6 +++++- compiler/rustc_typeck/src/check/mod.rs | 3 ++- compiler/rustc_typeck/src/expr_use_visitor.rs | 10 ++++++++-- .../closures/2229_closure_analysis/issue_88118.rs | 14 ++++++++++++++ src/test/ui/closures/issue-87814-1.rs | 8 ++++++++ src/test/ui/closures/issue-87814-2.rs | 11 +++++++++++ 6 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/closures/2229_closure_analysis/issue_88118.rs create mode 100644 src/test/ui/closures/issue-87814-1.rs create mode 100644 src/test/ui/closures/issue-87814-2.rs diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index 69a8970ae094e..8f6cdc7bb12a7 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -3,7 +3,9 @@ use rustc_infer::infer::type_variable::Diverging; use rustc_middle::ty::{self, Ty}; impl<'tcx> FnCtxt<'_, 'tcx> { - pub(super) fn type_inference_fallback(&self) { + /// Performs type inference fallback, returning true if any fallback + /// occurs. + pub(super) fn type_inference_fallback(&self) -> bool { // All type checking constraints were added, try to fallback unsolved variables. self.select_obligations_where_possible(false, |_| {}); let mut fallback_has_occurred = false; @@ -50,6 +52,8 @@ impl<'tcx> FnCtxt<'_, 'tcx> { // See if we can make any more progress. self.select_obligations_where_possible(fallback_has_occurred, |_| {}); + + fallback_has_occurred } // Tries to apply a fallback to `ty` if it is an unsolved variable. diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index ad7e96e2833b8..ff6cb35a75256 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -446,11 +446,12 @@ fn typeck_with_fallback<'tcx>( fcx }; - fcx.type_inference_fallback(); + let fallback_has_occurred = fcx.type_inference_fallback(); // Even though coercion casts provide type hints, we check casts after fallback for // backwards compatibility. This makes fallback a stronger type hint than a cast coercion. fcx.check_casts(); + fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); // Closure and generator analysis may run after fallback // because they don't constrain other type variables. diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index b0c95939bb77d..8d401b7f44442 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -243,7 +243,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self; let mut needs_to_be_read = false; for arm in arms.iter() { - return_if_err!(mc.cat_pattern(discr_place.clone(), &arm.pat, |place, pat| { + match mc.cat_pattern(discr_place.clone(), &arm.pat, |place, pat| { match &pat.kind { PatKind::Binding(.., opt_sub_pat) => { // If the opt_sub_pat is None, than the binding does not count as @@ -290,7 +290,13 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { // examined } } - })); + }) { + Ok(_) => (), + Err(_) => { + // If typeck failed, assume borrow is needed. + needs_to_be_read = true; + } + } } if needs_to_be_read { diff --git a/src/test/ui/closures/2229_closure_analysis/issue_88118.rs b/src/test/ui/closures/2229_closure_analysis/issue_88118.rs new file mode 100644 index 0000000000000..15c6ae4906c79 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/issue_88118.rs @@ -0,0 +1,14 @@ +// Regression test for #88118. Used to ICE. +// +// check-pass + +#![feature(capture_disjoint_fields)] + +fn foo(handler: impl FnOnce() -> MsU + Clone + 'static) { + Box::new(move |value| { + (|_| handler.clone()())(value); + None + }) as Box Option>; +} + +fn main() {} \ No newline at end of file diff --git a/src/test/ui/closures/issue-87814-1.rs b/src/test/ui/closures/issue-87814-1.rs new file mode 100644 index 0000000000000..5cf01ddf5d71b --- /dev/null +++ b/src/test/ui/closures/issue-87814-1.rs @@ -0,0 +1,8 @@ +// check-pass +fn main() { + let mut schema_all = vec![]; + (0..42).for_each(|_x| match Err(()) as Result<(), _> { + Ok(()) => schema_all.push(()), + Err(_) => (), + }); +} diff --git a/src/test/ui/closures/issue-87814-2.rs b/src/test/ui/closures/issue-87814-2.rs new file mode 100644 index 0000000000000..7a5facdac58c3 --- /dev/null +++ b/src/test/ui/closures/issue-87814-2.rs @@ -0,0 +1,11 @@ +// check-pass +#![feature(try_reserve)] + +fn main() { + let mut schema_all: (Vec, Vec) = (vec![], vec![]); + + let _c = || match schema_all.0.try_reserve(1) as Result<(), _> { + Ok(()) => (), + Err(_) => (), + }; +} From af15e529db5a7b31245d0b03d7af6dcb5ea21321 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Aug 2021 19:07:14 +0000 Subject: [PATCH 2/6] fix apparent typo in resolving variables --- compiler/rustc_typeck/src/check/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 9cbd3f7bb33a3..a2ae22eaf3931 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -1039,7 +1039,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let t_cast = self.to_ty_saving_user_provided_ty(t); let t_cast = self.resolve_vars_if_possible(t_cast); let t_expr = self.check_expr_with_expectation(e, ExpectCastableToType(t_cast)); - let t_cast = self.resolve_vars_if_possible(t_cast); + let t_expr = self.resolve_vars_if_possible(t_expr); // Eagerly check for some obvious errors. if t_expr.references_error() || t_cast.references_error() { From 754d51ebe40df99748bf224b4de7f43d5a345bd3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Aug 2021 19:14:06 +0000 Subject: [PATCH 3/6] useful debug printouts The changes to dumping expressions seem particularly useful --- compiler/rustc_typeck/src/check/_match.rs | 2 ++ compiler/rustc_typeck/src/check/cast.rs | 4 ++-- compiler/rustc_typeck/src/check/coercion.rs | 1 + compiler/rustc_typeck/src/check/expr.rs | 23 ++++++++++++++++++- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 1 + compiler/rustc_typeck/src/expr_use_visitor.rs | 13 +++-------- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 87fd8f1e03bb3..0397021c291b6 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -14,6 +14,7 @@ use rustc_trait_selection::traits::{ }; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { + #[instrument(skip(self), level="debug")] pub fn check_match( &self, expr: &'tcx hir::Expr<'tcx>, @@ -26,6 +27,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let acrb = arms_contain_ref_bindings(arms); let scrutinee_ty = self.demand_scrutinee_type(scrut, acrb, arms.is_empty()); + debug!(?scrutinee_ty); // If there are no arms, that is a diverging match; a special case. if arms.is_empty() { diff --git a/compiler/rustc_typeck/src/check/cast.rs b/compiler/rustc_typeck/src/check/cast.rs index 181972e3e7bad..14550690e63e0 100644 --- a/compiler/rustc_typeck/src/check/cast.rs +++ b/compiler/rustc_typeck/src/check/cast.rs @@ -51,6 +51,7 @@ use rustc_trait_selection::traits::error_reporting::report_object_safety_error; /// Reifies a cast check to be checked once we have full type information for /// a function context. +#[derive(Debug)] pub struct CastCheck<'tcx> { expr: &'tcx hir::Expr<'tcx>, expr_ty: Ty<'tcx>, @@ -603,12 +604,11 @@ impl<'a, 'tcx> CastCheck<'tcx> { }); } + #[instrument(skip(fcx), level = "debug")] pub fn check(mut self, fcx: &FnCtxt<'a, 'tcx>) { self.expr_ty = fcx.structurally_resolved_type(self.expr.span, self.expr_ty); self.cast_ty = fcx.structurally_resolved_type(self.cast_span, self.cast_ty); - debug!("check_cast({}, {:?} as {:?})", self.expr.hir_id, self.expr_ty, self.cast_ty); - if !fcx.type_is_known_to_be_sized_modulo_regions(self.cast_ty, self.span) { self.report_cast_to_unsized_type(fcx); } else if self.expr_ty.references_error() || self.cast_ty.references_error() { diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 0b4df8e6d3cdd..040524df9c7c9 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1328,6 +1328,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { /// The inner coercion "engine". If `expression` is `None`, this /// is a forced-unit case, and hence `expression_ty` must be /// `Nil`. + #[instrument(skip(self,fcx,augment_error,label_expression_as_expected), level="debug")] crate fn coerce_inner<'a>( &mut self, fcx: &FnCtxt<'a, 'tcx>, diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index a2ae22eaf3931..7880369e6c293 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -156,12 +156,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Note that inspecting a type's structure *directly* may expose the fact /// that there are actually multiple representations for `Error`, so avoid /// that when err needs to be handled differently. + #[instrument(skip(self), level="debug")] pub(super) fn check_expr_with_expectation( &self, expr: &'tcx hir::Expr<'tcx>, expected: Expectation<'tcx>, ) -> Ty<'tcx> { - debug!(">> type-checking: expected={:?}, expr={:?} ", expected, expr); + if self.tcx().sess.verbose() { + // make this code only run with -Zverbose because it is probably slow + if let Ok(lint_str) = self.tcx.sess.source_map().span_to_snippet(expr.span) { + if !lint_str.contains("\n") { + debug!("expr text: {}", lint_str); + } else { + let mut lines = lint_str.lines(); + if let Some(line0) = lines.next() { + let remaining_lines = lines.count(); + debug!("expr text: {}", line0); + debug!("expr text: ...(and {} more lines)", remaining_lines); + } + } + } + } // True if `expr` is a `Try::from_ok(())` that is a result of desugaring a try block // without the final expr (e.g. `try { return; }`). We don't want to generate an @@ -1049,6 +1064,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut deferred_cast_checks = self.deferred_cast_checks.borrow_mut(); match cast::CastCheck::new(self, e, t_expr, t_cast, t.span, expr.span) { Ok(cast_check) => { + debug!( + "check_expr_cast: deferring cast from {:?} to {:?}: {:?}", + t_cast, + t_expr, + cast_check, + ); deferred_cast_checks.push(cast_check); t_cast } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 9952413353fa9..b624e07374ec7 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -29,6 +29,7 @@ use std::slice; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub(in super::super) fn check_casts(&self) { let mut deferred_cast_checks = self.deferred_cast_checks.borrow_mut(); + debug!("FnCtxt::check_casts: {} deferred checks", deferred_cast_checks.len()); for cast in deferred_cast_checks.drain(..) { cast.check(self); } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 8d401b7f44442..08fb6793ac0bd 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -120,9 +120,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } + #[instrument(skip(self), level = "debug")] pub fn consume_body(&mut self, body: &hir::Body<'_>) { - debug!("consume_body(body={:?})", body); - for param in body.params { let param_ty = return_if_err!(self.mc.pat_ty_adjusted(¶m.pat)); debug!("consume_body: param_ty = {:?}", param_ty); @@ -243,7 +242,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self; let mut needs_to_be_read = false; for arm in arms.iter() { - match mc.cat_pattern(discr_place.clone(), &arm.pat, |place, pat| { + return_if_err!(mc.cat_pattern(discr_place.clone(), &arm.pat, |place, pat| { match &pat.kind { PatKind::Binding(.., opt_sub_pat) => { // If the opt_sub_pat is None, than the binding does not count as @@ -290,13 +289,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { // examined } } - }) { - Ok(_) => (), - Err(_) => { - // If typeck failed, assume borrow is needed. - needs_to_be_read = true; - } - } + })); } if needs_to_be_read { From e49323b07dd0a53318e2336f579fe9dd45c0741f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Aug 2021 19:53:18 +0000 Subject: [PATCH 4/6] add trailing newline --- src/test/ui/closures/2229_closure_analysis/issue_88118.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/closures/2229_closure_analysis/issue_88118.rs b/src/test/ui/closures/2229_closure_analysis/issue_88118.rs index 15c6ae4906c79..ea898bf1e6ffd 100644 --- a/src/test/ui/closures/2229_closure_analysis/issue_88118.rs +++ b/src/test/ui/closures/2229_closure_analysis/issue_88118.rs @@ -11,4 +11,4 @@ fn foo(handler: impl FnOnce() -> MsU + Clone + 'static) { }) as Box Option>; } -fn main() {} \ No newline at end of file +fn main() {} From ef2b9a406811fcfec34c8679099c409cc0a840b3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Aug 2021 22:21:21 +0000 Subject: [PATCH 5/6] x.py fmt --- compiler/rustc_typeck/src/check/_match.rs | 2 +- compiler/rustc_typeck/src/check/coercion.rs | 2 +- compiler/rustc_typeck/src/check/expr.rs | 8 +++----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 0397021c291b6..01227cad334fc 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -14,7 +14,7 @@ use rustc_trait_selection::traits::{ }; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { - #[instrument(skip(self), level="debug")] + #[instrument(skip(self), level = "debug")] pub fn check_match( &self, expr: &'tcx hir::Expr<'tcx>, diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 040524df9c7c9..17a81486048ed 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1328,7 +1328,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { /// The inner coercion "engine". If `expression` is `None`, this /// is a forced-unit case, and hence `expression_ty` must be /// `Nil`. - #[instrument(skip(self,fcx,augment_error,label_expression_as_expected), level="debug")] + #[instrument(skip(self, fcx, augment_error, label_expression_as_expected), level = "debug")] crate fn coerce_inner<'a>( &mut self, fcx: &FnCtxt<'a, 'tcx>, diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 7880369e6c293..eaf24552355d4 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -156,7 +156,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Note that inspecting a type's structure *directly* may expose the fact /// that there are actually multiple representations for `Error`, so avoid /// that when err needs to be handled differently. - #[instrument(skip(self), level="debug")] + #[instrument(skip(self), level = "debug")] pub(super) fn check_expr_with_expectation( &self, expr: &'tcx hir::Expr<'tcx>, @@ -176,7 +176,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } - } + } // True if `expr` is a `Try::from_ok(())` that is a result of desugaring a try block // without the final expr (e.g. `try { return; }`). We don't want to generate an @@ -1066,9 +1066,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Ok(cast_check) => { debug!( "check_expr_cast: deferring cast from {:?} to {:?}: {:?}", - t_cast, - t_expr, - cast_check, + t_cast, t_expr, cast_check, ); deferred_cast_checks.push(cast_check); t_cast From ec9531bcb09e52c5fbbabbedc8927b99f7e6fd9c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Aug 2021 22:25:55 +0000 Subject: [PATCH 6/6] fix test --- src/test/ui/closures/2229_closure_analysis/issue_88118.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/closures/2229_closure_analysis/issue_88118.rs b/src/test/ui/closures/2229_closure_analysis/issue_88118.rs index ea898bf1e6ffd..453b7e04a369b 100644 --- a/src/test/ui/closures/2229_closure_analysis/issue_88118.rs +++ b/src/test/ui/closures/2229_closure_analysis/issue_88118.rs @@ -2,6 +2,7 @@ // // check-pass +#![allow(incomplete_features)] #![feature(capture_disjoint_fields)] fn foo(handler: impl FnOnce() -> MsU + Clone + 'static) {