From 734209ed73507e214e49ef5cb7988af8901e74c7 Mon Sep 17 00:00:00 2001 From: ThePuzzlemaker Date: Mon, 6 Sep 2021 16:11:28 -0500 Subject: [PATCH 01/16] Normalize assoc types when checking ret ty of main This fixes #88609. Previously, the return type of `fn main()` would not have any associated type projections within normalized before checking if it implements the standard library trait `std::process::Termination`. This commit appears to fix it. This feels vaguely symptomatic of a problem in the underlying trait solving engine, but I am not sure how I would solve that. I am unsure why the example in #88609 with `assert_impl_termination` and `fn foo()` work, but simply `fn main()` doesn't. The way that I solved this is also probably not the best way to do this, so please let me know if there is a better way to do this. I have added a build-pass regression test for this issue. --- compiler/rustc_typeck/src/lib.rs | 18 +++++++++++++++++- src/test/ui/typeck/issue-88609.rs | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/typeck/issue-88609.rs diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs index 749f681e92ed1..e5d5ca3362bf4 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -109,6 +109,7 @@ use rustc_middle::util; use rustc_session::config::EntryFnType; use rustc_span::{symbol::sym, Span, DUMMY_SP}; use rustc_target::spec::abi::Abi; +use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _; use rustc_trait_selection::traits::{ self, ObligationCause, ObligationCauseCode, TraitEngine, TraitEngineExt as _, @@ -328,7 +329,22 @@ fn check_main_fn_ty(tcx: TyCtxt<'_>, main_def_id: DefId) { ObligationCauseCode::MainFunctionType, ); let mut fulfillment_cx = traits::FulfillmentContext::new(); - fulfillment_cx.register_bound(&infcx, ty::ParamEnv::empty(), return_ty, term_id, cause); + // normalize any potential projections in the return type, then add + // any possible obligations to the fulfillment context. + // HACK(ThePuzzlemaker) this feels symptomatic of a problem within + // checking trait fulfillment, not this here. I'm not sure why it + // works in the example in `fn test()` given in #88609? This also + // probably isn't the best way to do this. + let normalized = infcx.partially_normalize_associated_types_in( + cause.clone(), + ty::ParamEnv::empty(), + return_ty, + ); + let new_ty = normalized.value; + for obligation in normalized.obligations { + fulfillment_cx.register_predicate_obligation(&infcx, obligation); + } + fulfillment_cx.register_bound(&infcx, ty::ParamEnv::empty(), new_ty, term_id, cause); if let Err(err) = fulfillment_cx.select_all_or_error(&infcx) { infcx.report_fulfillment_errors(&err, None, false); error = true; diff --git a/src/test/ui/typeck/issue-88609.rs b/src/test/ui/typeck/issue-88609.rs new file mode 100644 index 0000000000000..dc459c885fa70 --- /dev/null +++ b/src/test/ui/typeck/issue-88609.rs @@ -0,0 +1,19 @@ +// Regression test for #88609: +// The return type for `main` is not normalized while checking if it implements +// the trait `std::process::Termination`. + +// build-pass + +trait Same { + type Output; +} + +impl Same for T { + type Output = T; +} + +type Unit = <() as Same>::Output; + +fn main() -> Result { + unimplemented!() +} From 33a28254f21c792cb0bda5427e66efa91b87dd4d Mon Sep 17 00:00:00 2001 From: ThePuzzlemaker Date: Mon, 6 Sep 2021 23:58:26 -0500 Subject: [PATCH 02/16] Clean up the fix a bit Use register_predicate_obligations rather than a for loop, since I didn't see that before. Also destructure in the `let` rather than projecting the fields individually --- compiler/rustc_typeck/src/lib.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs index e5d5ca3362bf4..e2cbaa4727289 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -335,16 +335,20 @@ fn check_main_fn_ty(tcx: TyCtxt<'_>, main_def_id: DefId) { // checking trait fulfillment, not this here. I'm not sure why it // works in the example in `fn test()` given in #88609? This also // probably isn't the best way to do this. - let normalized = infcx.partially_normalize_associated_types_in( - cause.clone(), + let InferOk { value: norm_return_ty, obligations } = infcx + .partially_normalize_associated_types_in( + cause.clone(), + ty::ParamEnv::empty(), + return_ty, + ); + fulfillment_cx.register_predicate_obligations(&infcx, obligations); + fulfillment_cx.register_bound( + &infcx, ty::ParamEnv::empty(), - return_ty, + norm_return_ty, + term_id, + cause, ); - let new_ty = normalized.value; - for obligation in normalized.obligations { - fulfillment_cx.register_predicate_obligation(&infcx, obligation); - } - fulfillment_cx.register_bound(&infcx, ty::ParamEnv::empty(), new_ty, term_id, cause); if let Err(err) = fulfillment_cx.select_all_or_error(&infcx) { infcx.report_fulfillment_errors(&err, None, false); error = true; From f1c8accf90d797ef32a56ee08ed7705a4b500c17 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Fri, 10 Sep 2021 19:36:51 +0200 Subject: [PATCH 03/16] Use `libc::sigaction()` instead of `sys::signal()` to prevent a deadlock --- library/std/src/sys/unix/process/process_unix.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 12edf04a4e2e9..13b384d8899f8 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -333,10 +333,9 @@ impl Command { let mut set = MaybeUninit::::uninit(); cvt(sigemptyset(set.as_mut_ptr()))?; cvt(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?; - let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); - if ret == libc::SIG_ERR { - return Err(io::Error::last_os_error()); - } + let mut action: libc::sigaction = mem::zeroed(); + action.sa_sigaction = libc::SIG_DFL; + cvt(libc::sigaction(libc::SIGPIPE, &action as *const _, ptr::null_mut()))?; } for callback in self.get_closures().iter_mut() { From 2bff77d255b9960489459157900a95a6e5a8279f Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sun, 12 Sep 2021 02:05:07 +0200 Subject: [PATCH 04/16] Fix suggestion for nested struct patterns --- compiler/rustc_passes/src/liveness.rs | 23 ++++++++++++------- .../ignore-nested-field-binding.fixed | 20 ++++++++++++++++ .../ignore-nested-field-binding.rs | 20 ++++++++++++++++ .../ignore-nested-field-binding.stderr | 20 ++++++++++++++++ 4 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/suggestions/ignore-nested-field-binding.fixed create mode 100644 src/test/ui/suggestions/ignore-nested-field-binding.rs create mode 100644 src/test/ui/suggestions/ignore-nested-field-binding.stderr diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index ab9bfea96943f..55de2e9d2f89b 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -265,12 +265,13 @@ impl IrMaps<'tcx> { self.capture_info_map.insert(hir_id, Rc::new(cs)); } - fn add_from_pat(&mut self, pat: &hir::Pat<'tcx>) { + fn collect_shorthand_field_ids(&self, pat: &hir::Pat<'tcx>) -> HirIdSet { // For struct patterns, take note of which fields used shorthand // (`x` rather than `x: x`). let mut shorthand_field_ids = HirIdSet::default(); let mut pats = VecDeque::new(); pats.push_back(pat); + while let Some(pat) = pats.pop_front() { use rustc_hir::PatKind::*; match &pat.kind { @@ -278,8 +279,10 @@ impl IrMaps<'tcx> { pats.extend(inner_pat.iter()); } Struct(_, fields, _) => { - let ids = fields.iter().filter(|f| f.is_shorthand).map(|f| f.pat.hir_id); - shorthand_field_ids.extend(ids); + let (short, not_short): (Vec<&_>, Vec<&_>) = + fields.iter().partition(|f| f.is_shorthand); + shorthand_field_ids.extend(short.iter().map(|f| f.pat.hir_id)); + pats.extend(not_short.iter().map(|f| f.pat)); } Ref(inner_pat, _) | Box(inner_pat) => { pats.push_back(inner_pat); @@ -296,6 +299,12 @@ impl IrMaps<'tcx> { } } + return shorthand_field_ids; + } + + fn add_from_pat(&mut self, pat: &hir::Pat<'tcx>) { + let shorthand_field_ids = self.collect_shorthand_field_ids(pat); + pat.each_binding(|_, hir_id, _, ident| { self.add_live_node_for_node(hir_id, VarDefNode(ident.span, hir_id)); self.add_variable(Local(LocalInfo { @@ -373,15 +382,13 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> { } fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { + let shorthand_field_ids = self.collect_shorthand_field_ids(param.pat); param.pat.each_binding(|_bm, hir_id, _x, ident| { let var = match param.pat.kind { - rustc_hir::PatKind::Struct(_, fields, _) => Local(LocalInfo { + rustc_hir::PatKind::Struct(..) => Local(LocalInfo { id: hir_id, name: ident.name, - is_shorthand: fields - .iter() - .find(|f| f.ident == ident) - .map_or(false, |f| f.is_shorthand), + is_shorthand: shorthand_field_ids.contains(&hir_id), }), _ => Param(hir_id, ident.name), }; diff --git a/src/test/ui/suggestions/ignore-nested-field-binding.fixed b/src/test/ui/suggestions/ignore-nested-field-binding.fixed new file mode 100644 index 0000000000000..1dc44838e8bb0 --- /dev/null +++ b/src/test/ui/suggestions/ignore-nested-field-binding.fixed @@ -0,0 +1,20 @@ +// Regression test for #88403, where prefixing with an underscore was +// erroneously suggested for a nested shorthand struct field binding. + +// run-rustfix +#![allow(unused)] +#![forbid(unused_variables)] + +struct Inner { i: i32 } +struct Outer { o: Inner } + +fn foo(Outer { o: Inner { i: _ } }: Outer) {} +//~^ ERROR: unused variable: `i` +//~| HELP: try ignoring the field + +fn main() { + let s = Outer { o: Inner { i: 42 } }; + let Outer { o: Inner { i: _ } } = s; + //~^ ERROR: unused variable: `i` + //~| HELP: try ignoring the field +} diff --git a/src/test/ui/suggestions/ignore-nested-field-binding.rs b/src/test/ui/suggestions/ignore-nested-field-binding.rs new file mode 100644 index 0000000000000..6dc0263ec9f2b --- /dev/null +++ b/src/test/ui/suggestions/ignore-nested-field-binding.rs @@ -0,0 +1,20 @@ +// Regression test for #88403, where prefixing with an underscore was +// erroneously suggested for a nested shorthand struct field binding. + +// run-rustfix +#![allow(unused)] +#![forbid(unused_variables)] + +struct Inner { i: i32 } +struct Outer { o: Inner } + +fn foo(Outer { o: Inner { i } }: Outer) {} +//~^ ERROR: unused variable: `i` +//~| HELP: try ignoring the field + +fn main() { + let s = Outer { o: Inner { i: 42 } }; + let Outer { o: Inner { i } } = s; + //~^ ERROR: unused variable: `i` + //~| HELP: try ignoring the field +} diff --git a/src/test/ui/suggestions/ignore-nested-field-binding.stderr b/src/test/ui/suggestions/ignore-nested-field-binding.stderr new file mode 100644 index 0000000000000..b2936a22a22f1 --- /dev/null +++ b/src/test/ui/suggestions/ignore-nested-field-binding.stderr @@ -0,0 +1,20 @@ +error: unused variable: `i` + --> $DIR/ignore-nested-field-binding.rs:11:27 + | +LL | fn foo(Outer { o: Inner { i } }: Outer) {} + | ^ help: try ignoring the field: `i: _` + | +note: the lint level is defined here + --> $DIR/ignore-nested-field-binding.rs:6:11 + | +LL | #![forbid(unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error: unused variable: `i` + --> $DIR/ignore-nested-field-binding.rs:17:28 + | +LL | let Outer { o: Inner { i } } = s; + | ^ help: try ignoring the field: `i: _` + +error: aborting due to 2 previous errors + From e3e5ae91d07f7d4256acac7abac6a204b6abc491 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 28 Sep 2021 21:17:57 -0700 Subject: [PATCH 05/16] Clean up unneeded explicit pointer cast The reference automatically coerces to a pointer. Writing an explicit cast here is slightly misleading because that's most commonly used when a pointer needs to be converted from one pointer type to another, e.g. `*const c_void` to `*const sigaction` or vice versa. --- library/std/src/sys/unix/process/process_unix.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 13b384d8899f8..caef9914ac2a7 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -335,7 +335,7 @@ impl Command { cvt(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?; let mut action: libc::sigaction = mem::zeroed(); action.sa_sigaction = libc::SIG_DFL; - cvt(libc::sigaction(libc::SIGPIPE, &action as *const _, ptr::null_mut()))?; + cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?; } for callback in self.get_closures().iter_mut() { From 65ef265c121c90e33dd9dd2dc3b919cf37209b71 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Fri, 1 Oct 2021 21:22:18 +0200 Subject: [PATCH 06/16] Call `libc::sigaction()` only on Android --- .../std/src/sys/unix/process/process_unix.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index caef9914ac2a7..5837c1553ec21 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -333,9 +333,20 @@ impl Command { let mut set = MaybeUninit::::uninit(); cvt(sigemptyset(set.as_mut_ptr()))?; cvt(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?; - let mut action: libc::sigaction = mem::zeroed(); - action.sa_sigaction = libc::SIG_DFL; - cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?; + + #[cfg(target_os = "android")] // see issue #88585 + { + let mut action: libc::sigaction = mem::zeroed(); + action.sa_sigaction = libc::SIG_DFL; + cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?; + } + #[cfg(not(target_os = "android"))] + { + let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); + if ret == libc::SIG_ERR { + return Err(io::Error::last_os_error()); + } + } } for callback in self.get_closures().iter_mut() { From e3996ffcb6bcb72d1c364ff19131fe60a9c77b9b Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sun, 3 Oct 2021 23:05:03 +0200 Subject: [PATCH 07/16] Fix Lower/UpperExp formatting for integers and precision zero --- library/core/src/fmt/num.rs | 3 +-- library/core/tests/fmt/num.rs | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/fmt/num.rs b/library/core/src/fmt/num.rs index db45640df48d6..05ca50a97a644 100644 --- a/library/core/src/fmt/num.rs +++ b/library/core/src/fmt/num.rs @@ -305,7 +305,6 @@ macro_rules! impl_Exp { n /= 10; exponent += 1; } - let trailing_zeros = exponent; let (added_precision, subtracted_precision) = match f.precision() { Some(fmt_prec) => { @@ -333,7 +332,7 @@ macro_rules! impl_Exp { n += 1; } } - (n, exponent, trailing_zeros, added_precision) + (n, exponent, exponent, added_precision) }; // 39 digits (worst case u128) + . = 40 diff --git a/library/core/tests/fmt/num.rs b/library/core/tests/fmt/num.rs index 275a1d062cafb..41eaf5a487880 100644 --- a/library/core/tests/fmt/num.rs +++ b/library/core/tests/fmt/num.rs @@ -146,6 +146,7 @@ fn test_format_int_exp_precision() { assert_eq!(format!("{:.1000e}", 1), format!("1.{}e0", "0".repeat(1000))); //test zero precision assert_eq!(format!("{:.0e}", 1), format!("1e0",)); + assert_eq!(format!("{:.0e}", 25), format!("3e1",)); //test padding with precision (and sign) assert_eq!(format!("{:+10.3e}", 1), " +1.000e0"); From 199b33f0d77c3fae1c2c982029df7168899f8aba Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sun, 3 Oct 2021 20:15:12 -0700 Subject: [PATCH 08/16] Use a test value that doesn't depend on the handling of even/odd rounding --- library/core/tests/fmt/num.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/tests/fmt/num.rs b/library/core/tests/fmt/num.rs index 41eaf5a487880..b958422d14f84 100644 --- a/library/core/tests/fmt/num.rs +++ b/library/core/tests/fmt/num.rs @@ -146,7 +146,7 @@ fn test_format_int_exp_precision() { assert_eq!(format!("{:.1000e}", 1), format!("1.{}e0", "0".repeat(1000))); //test zero precision assert_eq!(format!("{:.0e}", 1), format!("1e0",)); - assert_eq!(format!("{:.0e}", 25), format!("3e1",)); + assert_eq!(format!("{:.0e}", 35), format!("4e1",)); //test padding with precision (and sign) assert_eq!(format!("{:+10.3e}", 1), " +1.000e0"); From 9cb30f465ed07debfdb95bf2457423d1445e737d Mon Sep 17 00:00:00 2001 From: kadmin Date: Tue, 28 Sep 2021 04:10:33 +0000 Subject: [PATCH 09/16] Move generic error message to separate branches This decomposes an error message in generic constants into more specific branches, for better readability. --- .../src/traits/const_evaluatable.rs | 137 ++++++++++++------ ...y-size-in-generic-struct-param.full.stderr | 3 +- .../generic_const_exprs/closures.stderr | 3 +- .../generic_const_exprs/let-bindings.stderr | 6 +- .../generic_const_exprs/unused_expr.stderr | 9 +- .../issues/issue-67375.full.stderr | 2 +- .../issues/issue-67945-2.full.stderr | 3 +- 7 files changed, 111 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index 24fa5007f1ecd..25ec9682d8407 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -236,16 +236,27 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> { self.body.exprs[self.body_id].span } - fn error(&mut self, span: Option, msg: &str) -> Result { + fn error(&mut self, span: Span, msg: &str) -> Result { self.tcx .sess .struct_span_err(self.root_span(), "overly complex generic constant") - .span_label(span.unwrap_or(self.root_span()), msg) + .span_label(span, msg) .help("consider moving this anonymous constant into a `const` function") .emit(); Err(ErrorReported) } + fn maybe_supported_error(&mut self, span: Span, msg: &str) -> Result { + self.tcx + .sess + .struct_span_err(self.root_span(), "overly complex generic constant") + .span_label(span, msg) + .help("consider moving this anonymous constant into a `const` function") + .note("this operation may be supported in the future") + .emit(); + + Err(ErrorReported) + } fn new( tcx: TyCtxt<'tcx>, @@ -337,14 +348,14 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> { Ok(match &node.kind { // I dont know if handling of these 3 is correct &ExprKind::Scope { value, .. } => self.recurse_build(value)?, - &ExprKind::PlaceTypeAscription { source, .. } | - &ExprKind::ValueTypeAscription { source, .. } => self.recurse_build(source)?, + &ExprKind::PlaceTypeAscription { source, .. } + | &ExprKind::ValueTypeAscription { source, .. } => self.recurse_build(source)?, // subtle: associated consts are literals this arm handles // `::ASSOC` as well as `12` &ExprKind::Literal { literal, .. } => self.nodes.push(Node::Leaf(literal)), - ExprKind::Call { fun, args, .. } => { + ExprKind::Call { fun, args, .. } => { let fun = self.recurse_build(*fun)?; let mut new_args = Vec::::with_capacity(args.len()); @@ -353,7 +364,7 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> { } let new_args = self.tcx.arena.alloc_slice(&new_args); self.nodes.push(Node::FunctionCall(fun, new_args)) - }, + } &ExprKind::Binary { op, lhs, rhs } if Self::check_binop(op) => { let lhs = self.recurse_build(lhs)?; let rhs = self.recurse_build(rhs)?; @@ -362,7 +373,7 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> { &ExprKind::Unary { op, arg } if Self::check_unop(op) => { let arg = self.recurse_build(arg)?; self.nodes.push(Node::UnaryOp(op, arg)) - }, + } // This is necessary so that the following compiles: // // ``` @@ -370,60 +381,100 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> { // bar::<{ N + 1 }>(); // } // ``` - ExprKind::Block { body: thir::Block { stmts: box [], expr: Some(e), .. }} => self.recurse_build(*e)?, + ExprKind::Block { body: thir::Block { stmts: box [], expr: Some(e), .. } } => { + self.recurse_build(*e)? + } // `ExprKind::Use` happens when a `hir::ExprKind::Cast` is a // "coercion cast" i.e. using a coercion or is a no-op. // This is important so that `N as usize as usize` doesnt unify with `N as usize`. (untested) &ExprKind::Use { source } => { let arg = self.recurse_build(source)?; self.nodes.push(Node::Cast(abstract_const::CastKind::Use, arg, node.ty)) - }, + } &ExprKind::Cast { source } => { let arg = self.recurse_build(source)?; self.nodes.push(Node::Cast(abstract_const::CastKind::As, arg, node.ty)) - }, + } // FIXME(generic_const_exprs): We may want to support these. ExprKind::AddressOf { .. } | ExprKind::Borrow { .. } - | ExprKind::Deref { .. } - | ExprKind::Repeat { .. } - | ExprKind::Array { .. } - | ExprKind::Block { .. } - | ExprKind::NeverToAny { .. } - | ExprKind::Tuple { .. } - | ExprKind::Index { .. } - | ExprKind::Field { .. } - | ExprKind::ConstBlock { .. } - | ExprKind::Adt(_) => self.error( - Some(node.span), - "unsupported operation in generic constant, this may be supported in the future", + | ExprKind::Deref { .. } => self.maybe_supported_error( + node.span, + "dereferencing is not supported in generic constants", + )?, + ExprKind::Repeat { .. } | ExprKind::Array { .. } => self.maybe_supported_error( + node.span, + "array construction is not supported in generic constants", + )?, + ExprKind::Block { .. } => self.maybe_supported_error( + node.span, + "blocks are not supported in generic constant", + )?, + ExprKind::NeverToAny { .. } => self.maybe_supported_error( + node.span, + "converting nevers to any is not supported in generic constant", + )?, + ExprKind::Tuple { .. } => self.maybe_supported_error( + node.span, + "tuple construction is not supported in generic constants", + )?, + ExprKind::Index { .. } => self.maybe_supported_error( + node.span, + "indexing is not supported in generic constant", + )?, + ExprKind::Field { .. } => self.maybe_supported_error( + node.span, + "field access is not supported in generic constant", + )?, + ExprKind::ConstBlock { .. } => self.maybe_supported_error( + node.span, + "const blocks are not supported in generic constant", + )?, + ExprKind::Adt(_) => self.maybe_supported_error( + node.span, + "struct/enum construction is not supported in generic constants", + )?, + // dont know if this is correct + ExprKind::Pointer { .. } => + self.error(node.span, "pointer casts are not allowed in generic constants")?, + ExprKind::Yield { .. } => + self.error(node.span, "generator control flow is not allowed in generic constants")?, + ExprKind::Continue { .. } | ExprKind::Break { .. } | ExprKind::Loop { .. } => self + .error( + node.span, + "loops and loop control flow are not supported in generic constants", )?, + ExprKind::Box { .. } => + self.error(node.span, "allocations are not allowed in generic constants")?, + + ExprKind::Unary { .. } => unreachable!(), + // we handle valid unary/binary ops above + ExprKind::Binary { .. } => + self.error(node.span, "unsupported binary operation in generic constants")?, + ExprKind::LogicalOp { .. } => + self.error(node.span, "unsupported operation in generic constants, short-circuiting operations would imply control flow")?, + ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => { + self.error(node.span, "assignment is not supported in generic constants")? + } + ExprKind::Closure { .. } | ExprKind::Return { .. } => self.error( + node.span, + "closures and function keywords are not supported in generic constants", + )?, + // let expressions imply control flow + ExprKind::Match { .. } | ExprKind::If { .. } | ExprKind::Let { .. } => + self.error(node.span, "control flow is not supported in generic constants")?, + ExprKind::LlvmInlineAsm { .. } | ExprKind::InlineAsm { .. } => { + self.error(node.span, "assembly is not supported in generic constants")? + } - ExprKind::Match { .. } // we dont permit let stmts so `VarRef` and `UpvarRef` cant happen - | ExprKind::VarRef { .. } + ExprKind::VarRef { .. } | ExprKind::UpvarRef { .. } - | ExprKind::Closure { .. } - | ExprKind::Let { .. } // let expressions imply control flow - | ExprKind::Loop { .. } - | ExprKind::Assign { .. } | ExprKind::StaticRef { .. } - | ExprKind::LogicalOp { .. } - // we handle valid unary/binary ops above - | ExprKind::Unary { .. } - | ExprKind::Binary { .. } - | ExprKind::Break { .. } - | ExprKind::Continue { .. } - | ExprKind::If { .. } - | ExprKind::Pointer { .. } // dont know if this is correct - | ExprKind::ThreadLocalRef(_) - | ExprKind::LlvmInlineAsm { .. } - | ExprKind::Return { .. } - | ExprKind::Box { .. } // allocations not allowed in constants - | ExprKind::AssignOp { .. } - | ExprKind::InlineAsm { .. } - | ExprKind::Yield { .. } => self.error(Some(node.span), "unsupported operation in generic constant")?, + | ExprKind::ThreadLocalRef(_) => { + self.error(node.span, "unsupported operation in generic constant")? + } }) } } diff --git a/src/test/ui/const-generics/generic_const_exprs/array-size-in-generic-struct-param.full.stderr b/src/test/ui/const-generics/generic_const_exprs/array-size-in-generic-struct-param.full.stderr index 9b3c32a939779..041232e869079 100644 --- a/src/test/ui/const-generics/generic_const_exprs/array-size-in-generic-struct-param.full.stderr +++ b/src/test/ui/const-generics/generic_const_exprs/array-size-in-generic-struct-param.full.stderr @@ -10,9 +10,10 @@ error: overly complex generic constant --> $DIR/array-size-in-generic-struct-param.rs:19:15 | LL | arr: [u8; CFG.arr_size], - | ^^^^^^^^^^^^ unsupported operation in generic constant, this may be supported in the future + | ^^^^^^^^^^^^ field access is not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: aborting due to 2 previous errors diff --git a/src/test/ui/const-generics/generic_const_exprs/closures.stderr b/src/test/ui/const-generics/generic_const_exprs/closures.stderr index 95dae4ecc0431..0dfd804be41b4 100644 --- a/src/test/ui/const-generics/generic_const_exprs/closures.stderr +++ b/src/test/ui/const-generics/generic_const_exprs/closures.stderr @@ -4,9 +4,10 @@ error: overly complex generic constant LL | fn test() -> [u8; N + (|| 42)()] {} | ^^^^-------^^ | | - | unsupported operation in generic constant, this may be supported in the future + | dereferencing is not supported in generic constants | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: aborting due to previous error diff --git a/src/test/ui/const-generics/generic_const_exprs/let-bindings.stderr b/src/test/ui/const-generics/generic_const_exprs/let-bindings.stderr index c9f847995223a..5ebb4c3999c36 100644 --- a/src/test/ui/const-generics/generic_const_exprs/let-bindings.stderr +++ b/src/test/ui/const-generics/generic_const_exprs/let-bindings.stderr @@ -2,17 +2,19 @@ error: overly complex generic constant --> $DIR/let-bindings.rs:6:68 | LL | fn test() -> [u8; { let x = N; N + 1 }] where [u8; { let x = N; N + 1 }]: Default { - | ^^^^^^^^^^^^^^^^^^^^ unsupported operation in generic constant, this may be supported in the future + | ^^^^^^^^^^^^^^^^^^^^ blocks are not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: overly complex generic constant --> $DIR/let-bindings.rs:6:35 | LL | fn test() -> [u8; { let x = N; N + 1 }] where [u8; { let x = N; N + 1 }]: Default { - | ^^^^^^^^^^^^^^^^^^^^ unsupported operation in generic constant, this may be supported in the future + | ^^^^^^^^^^^^^^^^^^^^ blocks are not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: aborting due to 2 previous errors diff --git a/src/test/ui/const-generics/generic_const_exprs/unused_expr.stderr b/src/test/ui/const-generics/generic_const_exprs/unused_expr.stderr index 3da91b19a5ed9..df73acf53de65 100644 --- a/src/test/ui/const-generics/generic_const_exprs/unused_expr.stderr +++ b/src/test/ui/const-generics/generic_const_exprs/unused_expr.stderr @@ -2,25 +2,28 @@ error: overly complex generic constant --> $DIR/unused_expr.rs:4:34 | LL | fn add() -> [u8; { N + 1; 5 }] { - | ^^^^^^^^^^^^ unsupported operation in generic constant, this may be supported in the future + | ^^^^^^^^^^^^ blocks are not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: overly complex generic constant --> $DIR/unused_expr.rs:9:34 | LL | fn div() -> [u8; { N / 1; 5 }] { - | ^^^^^^^^^^^^ unsupported operation in generic constant, this may be supported in the future + | ^^^^^^^^^^^^ blocks are not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: overly complex generic constant --> $DIR/unused_expr.rs:16:38 | LL | fn fn_call() -> [u8; { foo(N); 5 }] { - | ^^^^^^^^^^^^^ unsupported operation in generic constant, this may be supported in the future + | ^^^^^^^^^^^^^ blocks are not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: aborting due to 3 previous errors diff --git a/src/test/ui/const-generics/issues/issue-67375.full.stderr b/src/test/ui/const-generics/issues/issue-67375.full.stderr index d7b52063dc4db..2ee5b6a587040 100644 --- a/src/test/ui/const-generics/issues/issue-67375.full.stderr +++ b/src/test/ui/const-generics/issues/issue-67375.full.stderr @@ -4,7 +4,7 @@ error: overly complex generic constant LL | inner: [(); { [|_: &T| {}; 0].len() }], | ^^---------------^^^^^^^^ | | - | unsupported operation in generic constant + | pointer casts are not allowed in generic constants | = help: consider moving this anonymous constant into a `const` function diff --git a/src/test/ui/const-generics/issues/issue-67945-2.full.stderr b/src/test/ui/const-generics/issues/issue-67945-2.full.stderr index fe0351a829220..cce85772aa4da 100644 --- a/src/test/ui/const-generics/issues/issue-67945-2.full.stderr +++ b/src/test/ui/const-generics/issues/issue-67945-2.full.stderr @@ -8,9 +8,10 @@ LL | | let x: Option> = None; LL | | LL | | 0 LL | | }], - | |_____^ unsupported operation in generic constant, this may be supported in the future + | |_____^ blocks are not supported in generic constant | = help: consider moving this anonymous constant into a `const` function + = note: this operation may be supported in the future error: aborting due to previous error From 32a5abc6fe0f705210a9697804f20fe4f32a4160 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 4 Oct 2021 11:05:48 -0500 Subject: [PATCH 10/16] Make `proc_macro_derive_resolution_fallback` a future-breakage lint When `cargo report future-incompatibilities` is stabilized (see #71249), this will cause dependencies that trigger this lint to be included in the report. --- compiler/rustc_lint_defs/src/builtin.rs | 1 + src/test/ui/proc-macro/generate-mod.stderr | 72 ++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 5830ce26fea3f..9d56bc193fb29 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1960,6 +1960,7 @@ declare_lint! { "detects proc macro derives using inaccessible names from parent modules", @future_incompatible = FutureIncompatibleInfo { reference: "issue #83583 ", + reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow, }; } diff --git a/src/test/ui/proc-macro/generate-mod.stderr b/src/test/ui/proc-macro/generate-mod.stderr index 285876aadb293..d6fd4baeb5ece 100644 --- a/src/test/ui/proc-macro/generate-mod.stderr +++ b/src/test/ui/proc-macro/generate-mod.stderr @@ -82,3 +82,75 @@ LL | #[derive(generate_mod::CheckDerive)] error: aborting due to 4 previous errors; 4 warnings emitted For more information about this error, try `rustc --explain E0412`. +Future incompatibility report: Future breakage diagnostic: +warning: cannot find type `FromOutside` in this scope + --> $DIR/generate-mod.rs:16:10 + | +LL | #[derive(generate_mod::CheckDerive)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ names from parent modules are not accessible without an explicit import + | + = note: `#[warn(proc_macro_derive_resolution_fallback)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83583 + = note: this warning originates in the derive macro `generate_mod::CheckDerive` (in Nightly builds, run with -Z macro-backtrace for more info) + +Future breakage diagnostic: +warning: cannot find type `OuterDerive` in this scope + --> $DIR/generate-mod.rs:16:10 + | +LL | #[derive(generate_mod::CheckDerive)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ names from parent modules are not accessible without an explicit import + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83583 + = note: this warning originates in the derive macro `generate_mod::CheckDerive` (in Nightly builds, run with -Z macro-backtrace for more info) + +Future breakage diagnostic: +warning: cannot find type `FromOutside` in this scope + --> $DIR/generate-mod.rs:23:14 + | +LL | #[derive(generate_mod::CheckDerive)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ names from parent modules are not accessible without an explicit import + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83583 + = note: this warning originates in the derive macro `generate_mod::CheckDerive` (in Nightly builds, run with -Z macro-backtrace for more info) + +Future breakage diagnostic: +warning: cannot find type `OuterDerive` in this scope + --> $DIR/generate-mod.rs:23:14 + | +LL | #[derive(generate_mod::CheckDerive)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ names from parent modules are not accessible without an explicit import + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83583 + = note: this warning originates in the derive macro `generate_mod::CheckDerive` (in Nightly builds, run with -Z macro-backtrace for more info) + +Future breakage diagnostic: +warning: cannot find type `FromOutside` in this scope + --> $DIR/generate-mod.rs:30:10 + | +LL | #[derive(generate_mod::CheckDeriveLint)] // OK, lint is suppressed + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ names from parent modules are not accessible without an explicit import + | +note: the lint level is defined here + --> $DIR/generate-mod.rs:30:10 + | +LL | #[derive(generate_mod::CheckDeriveLint)] // OK, lint is suppressed + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83583 + = note: this warning originates in the derive macro `generate_mod::CheckDeriveLint` (in Nightly builds, run with -Z macro-backtrace for more info) + +Future breakage diagnostic: +warning: cannot find type `OuterDeriveLint` in this scope + --> $DIR/generate-mod.rs:30:10 + | +LL | #[derive(generate_mod::CheckDeriveLint)] // OK, lint is suppressed + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ names from parent modules are not accessible without an explicit import + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83583 + = note: this warning originates in the derive macro `generate_mod::CheckDeriveLint` (in Nightly builds, run with -Z macro-backtrace for more info) + From 40fe064c63797f2be4cf37abdf30a324878e5929 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 1 Oct 2021 20:54:36 +0200 Subject: [PATCH 11/16] Add a check for duplicated doc aliases in unused lint --- compiler/rustc_passes/src/check_attr.rs | 42 +++++++++++++++++++++---- compiler/rustc_passes/src/lib.rs | 3 +- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 3e59fc4f55159..e7b2a018680ad 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -9,7 +9,7 @@ use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_ast::{ast, AttrStyle, Attribute, Lit, LitKind, NestedMetaItem}; -use rustc_data_structures::stable_set::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, struct_span_err, Applicability}; use rustc_feature::{AttributeType, BUILTIN_ATTRIBUTE_MAP}; use rustc_hir as hir; @@ -66,6 +66,7 @@ impl CheckAttrVisitor<'tcx> { target: Target, item: Option>, ) { + let mut doc_aliases = FxHashMap::default(); let mut is_valid = true; let mut specified_inline = None; let mut seen = FxHashSet::default(); @@ -79,7 +80,13 @@ impl CheckAttrVisitor<'tcx> { sym::track_caller => { self.check_track_caller(hir_id, &attr.span, attrs, span, target) } - sym::doc => self.check_doc_attrs(attr, hir_id, target, &mut specified_inline), + sym::doc => self.check_doc_attrs( + attr, + hir_id, + target, + &mut specified_inline, + &mut doc_aliases, + ), sym::no_link => self.check_no_link(hir_id, &attr, span, target), sym::export_name => self.check_export_name(hir_id, &attr, span, target), sym::rustc_layout_scalar_valid_range_start @@ -512,6 +519,7 @@ impl CheckAttrVisitor<'tcx> { hir_id: HirId, target: Target, is_list: bool, + aliases: &mut FxHashMap, ) -> bool { let tcx = self.tcx; let err_fn = move |span: Span, msg: &str| { @@ -582,17 +590,38 @@ impl CheckAttrVisitor<'tcx> { if &*item_name.as_str() == doc_alias { return err_fn(meta.span(), "is the same as the item's name"); } + let span = meta.span(); + if let Err(entry) = aliases.try_insert(doc_alias.to_owned(), span) { + self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, span, |lint| { + lint.build("doc alias is duplicated") + .span_label(*entry.entry.get(), "first defined here") + .emit(); + }); + } true } - fn check_doc_alias(&self, meta: &NestedMetaItem, hir_id: HirId, target: Target) -> bool { + fn check_doc_alias( + &self, + meta: &NestedMetaItem, + hir_id: HirId, + target: Target, + aliases: &mut FxHashMap, + ) -> bool { if let Some(values) = meta.meta_item_list() { let mut errors = 0; for v in values { match v.literal() { Some(l) => match l.kind { LitKind::Str(s, _) => { - if !self.check_doc_alias_value(v, &s.as_str(), hir_id, target, true) { + if !self.check_doc_alias_value( + v, + &s.as_str(), + hir_id, + target, + true, + aliases, + ) { errors += 1; } } @@ -621,7 +650,7 @@ impl CheckAttrVisitor<'tcx> { } errors == 0 } else if let Some(doc_alias) = meta.value_str().map(|s| s.to_string()) { - self.check_doc_alias_value(meta, &doc_alias, hir_id, target, false) + self.check_doc_alias_value(meta, &doc_alias, hir_id, target, false, aliases) } else { self.tcx .sess @@ -858,6 +887,7 @@ impl CheckAttrVisitor<'tcx> { hir_id: HirId, target: Target, specified_inline: &mut Option<(bool, Span)>, + aliases: &mut FxHashMap, ) -> bool { let mut is_valid = true; @@ -867,7 +897,7 @@ impl CheckAttrVisitor<'tcx> { match i_meta.name_or_empty() { sym::alias if !self.check_attr_not_crate_level(&meta, hir_id, "alias") - || !self.check_doc_alias(&meta, hir_id, target) => + || !self.check_doc_alias(&meta, hir_id, target, aliases) => { is_valid = false } diff --git a/compiler/rustc_passes/src/lib.rs b/compiler/rustc_passes/src/lib.rs index f583a5d58d540..4adec3c4f608d 100644 --- a/compiler/rustc_passes/src/lib.rs +++ b/compiler/rustc_passes/src/lib.rs @@ -9,8 +9,9 @@ #![feature(in_band_lifetimes)] #![feature(format_args_capture)] #![feature(iter_zip)] -#![feature(nll)] +#![feature(map_try_insert)] #![feature(min_specialization)] +#![feature(nll)] #![feature(try_blocks)] #![recursion_limit = "256"] From 013aa378f301af73116088bec86128ac7cbd8ab8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 1 Oct 2021 20:54:50 +0200 Subject: [PATCH 12/16] Add test for duplicated doc aliases --- src/test/ui/duplicate_doc_alias.rs | 9 +++++++++ src/test/ui/duplicate_doc_alias.stderr | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 src/test/ui/duplicate_doc_alias.rs create mode 100644 src/test/ui/duplicate_doc_alias.stderr diff --git a/src/test/ui/duplicate_doc_alias.rs b/src/test/ui/duplicate_doc_alias.rs new file mode 100644 index 0000000000000..a564ab64532b7 --- /dev/null +++ b/src/test/ui/duplicate_doc_alias.rs @@ -0,0 +1,9 @@ +#![deny(unused_attributes)] + +#[doc(alias = "A")] +#[doc(alias = "A")] //~ ERROR +#[doc(alias = "B")] +#[doc(alias("B"))] //~ ERROR +pub struct Foo; + +fn main() {} diff --git a/src/test/ui/duplicate_doc_alias.stderr b/src/test/ui/duplicate_doc_alias.stderr new file mode 100644 index 0000000000000..4b2dd1f8eb68e --- /dev/null +++ b/src/test/ui/duplicate_doc_alias.stderr @@ -0,0 +1,24 @@ +error: doc alias is duplicated + --> $DIR/duplicate_doc_alias.rs:4:7 + | +LL | #[doc(alias = "A")] + | ----------- first defined here +LL | #[doc(alias = "A")] + | ^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/duplicate_doc_alias.rs:1:9 + | +LL | #![deny(unused_attributes)] + | ^^^^^^^^^^^^^^^^^ + +error: doc alias is duplicated + --> $DIR/duplicate_doc_alias.rs:6:13 + | +LL | #[doc(alias = "B")] + | ----------- first defined here +LL | #[doc(alias("B"))] + | ^^^ + +error: aborting due to 2 previous errors + From 02c2a35c66fc8c4c9dab581e4baf8fde9672eb28 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 4 Oct 2021 14:09:19 -0700 Subject: [PATCH 13/16] Discuss field-sensitivity and enums in context of `MaybeLiveLocals` --- .../rustc_mir_dataflow/src/impls/liveness.rs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index 0039d3188d57a..3e2548845e20f 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -11,6 +11,37 @@ use crate::{AnalysisDomain, Backward, GenKill, GenKillAnalysis}; /// exist. See [this `mir-dataflow` test][flow-test] for an example. You almost never want to use /// this analysis without also looking at the results of [`MaybeBorrowedLocals`]. /// +/// ## Field-(in)sensitivity +/// +/// As the name suggests, this analysis is field insensitive. If a projection of a variable `x` is +/// assigned to (e.g. `x.0 = 42`), it does not "define" `x` as far as liveness is concerned. In fact, +/// such an assignment is currently marked as a "use" of `x` in an attempt to be maximally +/// conservative. +/// +/// ## Enums and `SetDiscriminant` +/// +/// Assigning a literal value to an `enum` (e.g. `Option`), does not result in a simple +/// assignment of the form `_1 = /*...*/` in the MIR. For example, the following assignment to `x`: +/// +/// ``` +/// x = Some(4); +/// ``` +/// +/// compiles to this MIR +/// +/// ``` +/// ((_1 as Some).0: i32) = const 4_i32; +/// discriminant(_1) = 1; +/// ``` +/// +/// However, `MaybeLiveLocals` **does** mark `x` (`_1`) as "killed" after a statement like this. +/// That's because it treats the `SetDiscriminant` operation as a definition of `x`, even though +/// the writes that actually initialized the locals happened earlier. +/// +/// This makes `MaybeLiveLocals` unsuitable for certain classes of optimization normally associated +/// with a live variables analysis, notably dead-store elimination. It's a dirty hack, but it works +/// okay for the generator state transform (currently the main consumuer of this analysis). +/// /// [`MaybeBorrowedLocals`]: super::MaybeBorrowedLocals /// [flow-test]: https://github.com/rust-lang/rust/blob/a08c47310c7d49cbdc5d7afb38408ba519967ecd/src/test/ui/mir-dataflow/liveness-ptr.rs /// [liveness]: https://en.wikipedia.org/wiki/Live_variable_analysis From 9f9f7f695a793c5ef27d219dbd00c66810f34e92 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 4 Oct 2021 14:10:30 -0700 Subject: [PATCH 14/16] Ensure that `MaybeLiveLocals` works with simple sum-type assignments --- src/test/ui/mir-dataflow/liveness-enum.rs | 22 +++++++++++++++++++ src/test/ui/mir-dataflow/liveness-enum.stderr | 10 +++++++++ 2 files changed, 32 insertions(+) create mode 100644 src/test/ui/mir-dataflow/liveness-enum.rs create mode 100644 src/test/ui/mir-dataflow/liveness-enum.stderr diff --git a/src/test/ui/mir-dataflow/liveness-enum.rs b/src/test/ui/mir-dataflow/liveness-enum.rs new file mode 100644 index 0000000000000..5eb04ae8c8d37 --- /dev/null +++ b/src/test/ui/mir-dataflow/liveness-enum.rs @@ -0,0 +1,22 @@ +#![feature(core_intrinsics, rustc_attrs)] + +use std::intrinsics::rustc_peek; + +#[rustc_mir(rustc_peek_liveness, stop_after_dataflow)] +fn foo() -> Option { + let mut x = None; + + // `x` is live here since it is used in the next statement... + rustc_peek(x); + + dbg!(x); + + // But not here, since it is overwritten below + rustc_peek(x); //~ ERROR rustc_peek: bit not set + + x = Some(4); + + x +} + +fn main() {} diff --git a/src/test/ui/mir-dataflow/liveness-enum.stderr b/src/test/ui/mir-dataflow/liveness-enum.stderr new file mode 100644 index 0000000000000..483944d731aed --- /dev/null +++ b/src/test/ui/mir-dataflow/liveness-enum.stderr @@ -0,0 +1,10 @@ +error: rustc_peek: bit not set + --> $DIR/liveness-enum.rs:15:5 + | +LL | rustc_peek(x); + | ^^^^^^^^^^^^^ + +error: stop_after_dataflow ended compilation + +error: aborting due to 2 previous errors + From c35a700be2aebc0449a16fa1a09d7667aa209230 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Mon, 4 Oct 2021 19:23:13 -0700 Subject: [PATCH 15/16] Make an initial guess for metadata size to reduce buffer resizes --- compiler/rustc_metadata/src/locator.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index b6922e0d72a06..5bd9980566b65 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -740,7 +740,9 @@ fn get_metadata_section( // Header is okay -> inflate the actual metadata let compressed_bytes = &buf[header_len..]; debug!("inflating {} bytes of compressed metadata", compressed_bytes.len()); - let mut inflated = Vec::new(); + // Assume the decompressed data will be at least the size of the compressed data, so we + // don't have to grow the buffer as much. + let mut inflated = Vec::with_capacity(compressed_bytes.len()); match FrameDecoder::new(compressed_bytes).read_to_end(&mut inflated) { Ok(_) => rustc_erase_owner!(OwningRef::new(inflated).map_owner_box()), Err(_) => { From 4ec0377d6a9118b41df4fb587dc0b7d1fc53656f Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Tue, 5 Oct 2021 15:15:24 +0200 Subject: [PATCH 16/16] for signed overflowing remainder, delay comparing lhs with MIN Since the wrapped remainder is going to be 0 for all cases when the rhs is -1, there is no need to divide in this case. Comparing the lhs with MIN is only done for the overflow bool. In particular, this results in better code generation for wrapping remainder, which discards the overflow bool completely. --- library/core/src/num/int_macros.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/core/src/num/int_macros.rs b/library/core/src/num/int_macros.rs index f1dcdf2c1aa04..9d46948ce02af 100644 --- a/library/core/src/num/int_macros.rs +++ b/library/core/src/num/int_macros.rs @@ -1533,9 +1533,8 @@ macro_rules! int_impl { #[must_use = "this returns the result of the operation, \ without modifying the original"] pub const fn overflowing_rem(self, rhs: Self) -> (Self, bool) { - // Using `&` helps LLVM see that it is the same check made in division. - if unlikely!((self == Self::MIN) & (rhs == -1)) { - (0, true) + if unlikely!(rhs == -1) { + (0, self == Self::MIN) } else { (self % rhs, false) } @@ -1565,9 +1564,8 @@ macro_rules! int_impl { without modifying the original"] #[inline] pub const fn overflowing_rem_euclid(self, rhs: Self) -> (Self, bool) { - // Using `&` helps LLVM see that it is the same check made in division. - if unlikely!((self == Self::MIN) & (rhs == -1)) { - (0, true) + if unlikely!(rhs == -1) { + (0, self == Self::MIN) } else { (self.rem_euclid(rhs), false) }