From 9534b5863f0a6e41c6566795995219bd06ec4bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 30 Oct 2019 21:24:08 -0700 Subject: [PATCH 01/12] Point at where clauses where the associated item was restricted --- src/librustc/traits/error_reporting.rs | 9 +- src/librustc/traits/mod.rs | 9 +- src/librustc/traits/structural_impls.rs | 2 +- src/librustc/ty/wf.rs | 101 +++++++++++++++--- .../point-at-type-on-obligation-failure-2.rs | 16 +++ ...int-at-type-on-obligation-failure-2.stderr | 28 ++++- 6 files changed, 147 insertions(+), 18 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index b55c0f29a70ec..20eb2a7e209fc 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2225,11 +2225,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ); } } - ObligationCauseCode::AssocTypeBound(impl_span, orig) => { - err.span_label(orig, "associated type defined here"); - if let Some(sp) = impl_span { + ObligationCauseCode::AssocTypeBound(ref data) => { + err.span_label(data.original, "associated type defined here"); + if let Some(sp) = data.impl_span { err.span_label(sp, "in this `impl` item"); } + for sp in &data.bounds { + err.span_label(*sp, "restricted in this bound"); + } } } } diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index b8275299562ce..a29d8c66d811d 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -276,7 +276,14 @@ pub enum ObligationCauseCode<'tcx> { /// #[feature(trivial_bounds)] is not enabled TrivialBound, - AssocTypeBound(/*impl*/ Option, /*original*/ Span), + AssocTypeBound(Box), +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct AssocTypeBoundData { + pub impl_span: Option, + pub original: Span, + pub bounds: Vec, } // `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger. diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 109e884f8bd16..59f2bb3754803 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -549,7 +549,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> { super::MethodReceiver => Some(super::MethodReceiver), super::BlockTailExpression(id) => Some(super::BlockTailExpression(id)), super::TrivialBound => Some(super::TrivialBound), - super::AssocTypeBound(impl_sp, sp) => Some(super::AssocTypeBound(impl_sp, sp)), + super::AssocTypeBound(ref data) => Some(super::AssocTypeBound(data.clone())), } } } diff --git a/src/librustc/ty/wf.rs b/src/librustc/ty/wf.rs index 4ea01bf96476a..f9e7a8030a6fc 100644 --- a/src/librustc/ty/wf.rs +++ b/src/librustc/ty/wf.rs @@ -2,9 +2,10 @@ use crate::hir; use crate::hir::def_id::DefId; use crate::infer::InferCtxt; use crate::ty::subst::SubstsRef; -use crate::traits; +use crate::traits::{self, AssocTypeBoundData}; use crate::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable}; use std::iter::once; +use syntax::symbol::{kw, Ident}; use syntax_pos::Span; use crate::middle::lang_items; use crate::mir::interpret::ConstValue; @@ -176,6 +177,23 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { pred: &ty::Predicate<'_>, trait_assoc_items: ty::AssocItemsIterator<'_>, | { + let trait_item = tcx.hir().as_local_hir_id(trait_ref.def_id).and_then(|trait_id| { + tcx.hir().find(trait_id) + }); + let (trait_name, trait_generics) = match trait_item { + Some(hir::Node::Item(hir::Item { + ident, + kind: hir::ItemKind::Trait(.., generics, _, _), + .. + })) | + Some(hir::Node::Item(hir::Item { + ident, + kind: hir::ItemKind::TraitAlias(generics, _), + .. + })) => (Some(ident), Some(generics)), + _ => (None, None), + }; + let item_span = item.map(|i| tcx.sess.source_map().def_span(i.span)); match pred { ty::Predicate::Projection(proj) => { @@ -226,10 +244,11 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { item.ident == trait_assoc_item.ident }).next() { cause.span = impl_item.span; - cause.code = traits::AssocTypeBound( - item_span, - trait_assoc_item.ident.span, - ); + cause.code = traits::AssocTypeBound(Box::new(AssocTypeBoundData { + impl_span: item_span, + original: trait_assoc_item.ident.span, + bounds: vec![], + })); } } } @@ -251,14 +270,13 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { // LL | type Assoc = bool; // | ^^^^^^^^^^^^^^^^^^ the trait `Bar` is not implemented for `bool` // - // FIXME: if the obligation comes from the where clause in the `trait`, we - // should point at it: + // If the obligation comes from the where clause in the `trait`, we point at it: // // error[E0277]: the trait bound `bool: Bar` is not satisfied // --> $DIR/point-at-type-on-obligation-failure-2.rs:8:5 // | // | trait Foo where >::Assoc: Bar { - // | -------------------------- obligation set here + // | -------------------------- restricted in this bound // LL | type Assoc; // | ----- associated type defined here // ... @@ -278,11 +296,17 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { .next() .map(|impl_item| (impl_item, trait_assoc_item))) { + let bounds = trait_generics.map(|generics| get_generic_bound_spans( + &generics, + trait_name, + trait_assoc_item.ident, + )).unwrap_or_else(Vec::new); cause.span = impl_item.span; - cause.code = traits::AssocTypeBound( - item_span, - trait_assoc_item.ident.span, - ); + cause.code = traits::AssocTypeBound(Box::new(AssocTypeBoundData { + impl_span: item_span, + original: trait_assoc_item.ident.span, + bounds, + })); } } } @@ -666,3 +690,56 @@ pub fn object_region_bounds<'tcx>( tcx.required_region_bounds(open_ty, predicates) } + +/// Find the span of a generic bound affecting an associated type. +fn get_generic_bound_spans( + generics: &hir::Generics, + trait_name: Option<&Ident>, + assoc_item_name: Ident, +) -> Vec { + let mut bounds = vec![]; + for clause in generics.where_clause.predicates.iter() { + if let hir::WherePredicate::BoundPredicate(pred) = clause { + match &pred.bounded_ty.kind { + hir::TyKind::Path(hir::QPath::Resolved(Some(ty), path)) => { + let mut s = path.segments.iter(); + if let (a, Some(b), None) = (s.next(), s.next(), s.next()) { + if a.map(|s| &s.ident) == trait_name + && b.ident == assoc_item_name + && is_self_path(&ty.kind) + { + // `::Bar` + bounds.push(pred.span); + } + } + } + hir::TyKind::Path(hir::QPath::TypeRelative(ty, segment)) => { + if segment.ident == assoc_item_name { + if is_self_path(&ty.kind) { + // `Self::Bar` + bounds.push(pred.span); + } + } + } + _ => {} + } + } + } + bounds +} + +fn is_self_path(kind: &hir::TyKind) -> bool { + match kind { + hir::TyKind::Path(hir::QPath::Resolved(None, path)) => { + let mut s = path.segments.iter(); + if let (Some(segment), None) = (s.next(), s.next()) { + if segment.ident.name == kw::SelfUpper { + // `type(Self)` + return true; + } + } + } + _ => {} + } + false +} diff --git a/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.rs b/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.rs index 9360d96f05e17..67b7c78071c37 100644 --- a/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.rs +++ b/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.rs @@ -8,4 +8,20 @@ impl Foo for () { type Assoc = bool; //~ ERROR the trait bound `bool: Bar` is not satisfied } +trait Baz where Self::Assoc: Bar { + type Assoc; +} + +impl Baz for () { + type Assoc = bool; //~ ERROR the trait bound `bool: Bar` is not satisfied +} + +trait Bat where ::Assoc: Bar { + type Assoc; +} + +impl Bat for () { + type Assoc = bool; //~ ERROR the trait bound `bool: Bar` is not satisfied +} + fn main() {} diff --git a/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.stderr b/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.stderr index f1a2e343a7ec3..072e9dad062e0 100644 --- a/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.stderr +++ b/src/test/ui/associated-types/point-at-type-on-obligation-failure-2.stderr @@ -9,6 +9,32 @@ LL | impl Foo for () { LL | type Assoc = bool; | ^^^^^^^^^^^^^^^^^^ the trait `Bar` is not implemented for `bool` -error: aborting due to previous error +error[E0277]: the trait bound `bool: Bar` is not satisfied + --> $DIR/point-at-type-on-obligation-failure-2.rs:16:5 + | +LL | trait Baz where Self::Assoc: Bar { + | ---------------- restricted in this bound +LL | type Assoc; + | ----- associated type defined here +... +LL | impl Baz for () { + | --------------- in this `impl` item +LL | type Assoc = bool; + | ^^^^^^^^^^^^^^^^^^ the trait `Bar` is not implemented for `bool` + +error[E0277]: the trait bound `bool: Bar` is not satisfied + --> $DIR/point-at-type-on-obligation-failure-2.rs:24:5 + | +LL | trait Bat where ::Assoc: Bar { + | ------------------------- restricted in this bound +LL | type Assoc; + | ----- associated type defined here +... +LL | impl Bat for () { + | --------------- in this `impl` item +LL | type Assoc = bool; + | ^^^^^^^^^^^^^^^^^^ the trait `Bar` is not implemented for `bool` + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0277`. From 08b235b5bed1a53311da34fa12966cd42a2a5abe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 5 Nov 2019 11:55:00 -0800 Subject: [PATCH 02/12] Point at formatting descriptor string when it is invalid When a formatting string contains an invalid descriptor, point at it instead of the argument: ``` error: unknown format trait `foo` --> $DIR/ifmt-bad-arg.rs:86:17 | LL | println!("{:foo}", 1); | ^^^ | = note: the only appropriate formatting traits are: - ``, which uses the `Display` trait - `?`, which uses the `Debug` trait - `e`, which uses the `LowerExp` trait - `E`, which uses the `UpperExp` trait - `o`, which uses the `Octal` trait - `p`, which uses the `Pointer` trait - `b`, which uses the `Binary` trait - `x`, which uses the `LowerHex` trait - `X`, which uses the `UpperHex` trait ``` --- src/libfmt_macros/lib.rs | 9 ++++ src/libsyntax_ext/format.rs | 67 ++++++++++++------------ src/test/ui/if/ifmt-bad-arg.stderr | 4 +- src/test/ui/if/ifmt-unknown-trait.stderr | 4 +- 4 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index d22420e76dcd4..3896612ea0388 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -74,6 +74,8 @@ pub struct FormatSpec<'a> { /// this argument, this can be empty or any number of characters, although /// it is required to be one word. pub ty: &'a str, + /// The span of the descriptor string (for diagnostics). + pub ty_span: Option, } /// Enum describing where an argument for a format can be located. @@ -475,6 +477,7 @@ impl<'a> Parser<'a> { width: CountImplied, width_span: None, ty: &self.input[..0], + ty_span: None, }; if !self.consume(':') { return spec; @@ -548,6 +551,7 @@ impl<'a> Parser<'a> { spec.precision_span = sp; } } + let ty_span_start = self.cur.peek().map(|(pos, _)| *pos); // Optional radix followed by the actual format specifier if self.consume('x') { if self.consume('?') { @@ -567,6 +571,11 @@ impl<'a> Parser<'a> { spec.ty = "?"; } else { spec.ty = self.word(); + let ty_span_end = self.cur.peek().map(|(pos, _)| *pos); + let this = self; + spec.ty_span = ty_span_start + .and_then(|s| ty_span_end.map(|e| (s, e))) + .map(|(start, end)| this.to_span_index(start).to(this.to_span_index(end))); } spec } diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 37310f46f7eed..b8d053a2162de 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -21,7 +21,7 @@ use std::collections::hash_map::Entry; #[derive(PartialEq)] enum ArgumentType { - Placeholder(String), + Placeholder(&'static str), Count, } @@ -244,7 +244,36 @@ impl<'a, 'b> Context<'a, 'b> { parse::ArgumentNamed(s) => Named(s), }; - let ty = Placeholder(arg.format.ty.to_string()); + let ty = Placeholder(match &arg.format.ty[..] { + "" => "Display", + "?" => "Debug", + "e" => "LowerExp", + "E" => "UpperExp", + "o" => "Octal", + "p" => "Pointer", + "b" => "Binary", + "x" => "LowerHex", + "X" => "UpperHex", + _ => { + let fmtsp = self.fmtsp; + let mut err = self.ecx.struct_span_err( + arg.format.ty_span.map(|sp| fmtsp.from_inner(sp)).unwrap_or(fmtsp), + &format!("unknown format trait `{}`", arg.format.ty), + ); + err.note("the only appropriate formatting traits are:\n\ + - ``, which uses the `Display` trait\n\ + - `?`, which uses the `Debug` trait\n\ + - `e`, which uses the `LowerExp` trait\n\ + - `E`, which uses the `UpperExp` trait\n\ + - `o`, which uses the `Octal` trait\n\ + - `p`, which uses the `Pointer` trait\n\ + - `b`, which uses the `Binary` trait\n\ + - `x`, which uses the `LowerHex` trait\n\ + - `X`, which uses the `UpperHex` trait"); + err.emit(); + "" + } + }); self.verify_arg_type(pos, ty); self.curpiece += 1; } @@ -588,6 +617,7 @@ impl<'a, 'b> Context<'a, 'b> { width: parse::CountImplied, width_span: None, ty: arg.format.ty, + ty_span: arg.format.ty_span, }, }; @@ -759,37 +789,8 @@ impl<'a, 'b> Context<'a, 'b> { sp = ecx.with_def_site_ctxt(sp); let arg = ecx.expr_ident(sp, arg); let trait_ = match *ty { - Placeholder(ref tyname) => { - match &tyname[..] { - "" => "Display", - "?" => "Debug", - "e" => "LowerExp", - "E" => "UpperExp", - "o" => "Octal", - "p" => "Pointer", - "b" => "Binary", - "x" => "LowerHex", - "X" => "UpperHex", - _ => { - let mut err = ecx.struct_span_err( - sp, - &format!("unknown format trait `{}`", *tyname), - ); - err.note("the only appropriate formatting traits are:\n\ - - ``, which uses the `Display` trait\n\ - - `?`, which uses the `Debug` trait\n\ - - `e`, which uses the `LowerExp` trait\n\ - - `E`, which uses the `UpperExp` trait\n\ - - `o`, which uses the `Octal` trait\n\ - - `p`, which uses the `Pointer` trait\n\ - - `b`, which uses the `Binary` trait\n\ - - `x`, which uses the `LowerHex` trait\n\ - - `X`, which uses the `UpperHex` trait"); - err.emit(); - return DummyResult::raw_expr(sp, true); - } - } - } + Placeholder(trait_) if trait_ == "" => return DummyResult::raw_expr(sp, true), + Placeholder(trait_) => trait_, Count => { let path = ecx.std_path(&[sym::fmt, sym::ArgumentV1, sym::from_usize]); return ecx.expr_call_global(macsp, path, vec![arg]); diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr index c58cbc312335a..7a76f0e7c4789 100644 --- a/src/test/ui/if/ifmt-bad-arg.stderr +++ b/src/test/ui/if/ifmt-bad-arg.stderr @@ -257,10 +257,10 @@ LL | println!("{} {:07$} {}", 1, 3.2, 4); = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html error: unknown format trait `foo` - --> $DIR/ifmt-bad-arg.rs:86:24 + --> $DIR/ifmt-bad-arg.rs:86:17 | LL | println!("{:foo}", 1); - | ^ + | ^^^ | = note: the only appropriate formatting traits are: - ``, which uses the `Display` trait diff --git a/src/test/ui/if/ifmt-unknown-trait.stderr b/src/test/ui/if/ifmt-unknown-trait.stderr index 7853b5ca0c9a6..459432bf4e426 100644 --- a/src/test/ui/if/ifmt-unknown-trait.stderr +++ b/src/test/ui/if/ifmt-unknown-trait.stderr @@ -1,8 +1,8 @@ error: unknown format trait `notimplemented` - --> $DIR/ifmt-unknown-trait.rs:2:34 + --> $DIR/ifmt-unknown-trait.rs:2:16 | LL | format!("{:notimplemented}", "3"); - | ^^^ + | ^^^^^^^^^^^^^^ | = note: the only appropriate formatting traits are: - ``, which uses the `Display` trait From c271db284bea3882fe033afb1cd4c6f370c69dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 5 Nov 2019 12:08:22 -0800 Subject: [PATCH 03/12] Provide structured suggestions for valid formatting descriptors --- src/libsyntax_ext/format.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index b8d053a2162de..b47038397b80b 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -256,8 +256,9 @@ impl<'a, 'b> Context<'a, 'b> { "X" => "UpperHex", _ => { let fmtsp = self.fmtsp; + let sp = arg.format.ty_span.map(|sp| fmtsp.from_inner(sp)); let mut err = self.ecx.struct_span_err( - arg.format.ty_span.map(|sp| fmtsp.from_inner(sp)).unwrap_or(fmtsp), + sp.unwrap_or(fmtsp), &format!("unknown format trait `{}`", arg.format.ty), ); err.note("the only appropriate formatting traits are:\n\ @@ -270,6 +271,26 @@ impl<'a, 'b> Context<'a, 'b> { - `b`, which uses the `Binary` trait\n\ - `x`, which uses the `LowerHex` trait\n\ - `X`, which uses the `UpperHex` trait"); + if let Some(sp) = sp { + for (fmt, name) in &[ + ("", "Display"), + ("?", "Debug"), + ("e", "LowerExp"), + ("E", "UpperExp"), + ("o", "Octal"), + ("p", "Pointer"), + ("b", "Binary"), + ("x", "LowerHex"), + ("X", "UpperHex"), + ] { + err.tool_only_span_suggestion( + sp, + &format!("use the `{}` trait", name), + fmt.to_string(), + Applicability::MaybeIncorrect, + ); + } + } err.emit(); "" } From e648aa8e89f4d30dd1e7242de0ce1fd660bfd422 Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Wed, 4 Sep 2019 20:40:18 -0500 Subject: [PATCH 04/12] Fix C aggregate-passing ABI on powerpc The existing code (which looks like it was copied from MIPS) passes aggregates by value in registers. This is wrong. According to the SVR4 powerpc psABI, all aggregates are passed indirectly. See #64259 for more discussion, which addresses the ABI for the special case of ZSTs (empty structs). --- src/librustc_target/abi/call/mod.rs | 2 +- src/librustc_target/abi/call/powerpc.rs | 41 ++++++------------------- 2 files changed, 11 insertions(+), 32 deletions(-) diff --git a/src/librustc_target/abi/call/mod.rs b/src/librustc_target/abi/call/mod.rs index 396b962003803..33335f462a7ae 100644 --- a/src/librustc_target/abi/call/mod.rs +++ b/src/librustc_target/abi/call/mod.rs @@ -554,7 +554,7 @@ impl<'a, Ty> FnAbi<'a, Ty> { "arm" => arm::compute_abi_info(cx, self), "mips" => mips::compute_abi_info(cx, self), "mips64" => mips64::compute_abi_info(cx, self), - "powerpc" => powerpc::compute_abi_info(cx, self), + "powerpc" => powerpc::compute_abi_info(self), "powerpc64" => powerpc64::compute_abi_info(cx, self), "s390x" => s390x::compute_abi_info(cx, self), "msp430" => msp430::compute_abi_info(self), diff --git a/src/librustc_target/abi/call/powerpc.rs b/src/librustc_target/abi/call/powerpc.rs index b2c8d26ff1f86..740bd7222f237 100644 --- a/src/librustc_target/abi/call/powerpc.rs +++ b/src/librustc_target/abi/call/powerpc.rs @@ -1,49 +1,28 @@ -use crate::abi::call::{ArgAbi, FnAbi, Reg, Uniform}; -use crate::abi::{HasDataLayout, LayoutOf, Size, TyLayoutMethods}; +use crate::abi::call::{ArgAbi, FnAbi}; -fn classify_ret<'a, Ty, C>(cx: &C, ret: &mut ArgAbi<'_, Ty>, offset: &mut Size) - where Ty: TyLayoutMethods<'a, C>, C: LayoutOf + HasDataLayout -{ - if !ret.layout.is_aggregate() { - ret.extend_integer_width_to(32); - } else { +fn classify_ret(ret: &mut ArgAbi<'_, Ty>) { + if ret.layout.is_aggregate() { ret.make_indirect(); - *offset += cx.data_layout().pointer_size; + } else { + ret.extend_integer_width_to(32); } } -fn classify_arg<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size) - where Ty: TyLayoutMethods<'a, C>, C: LayoutOf + HasDataLayout -{ - let dl = cx.data_layout(); - let size = arg.layout.size; - let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi; - +fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { if arg.layout.is_aggregate() { - arg.cast_to(Uniform { - unit: Reg::i32(), - total: size - }); - if !offset.is_aligned(align) { - arg.pad_with(Reg::i32()); - } + arg.make_indirect(); } else { arg.extend_integer_width_to(32); } - - *offset = offset.align_to(align) + size.align_to(align); } -pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'_, Ty>) - where Ty: TyLayoutMethods<'a, C>, C: LayoutOf + HasDataLayout -{ - let mut offset = Size::ZERO; +pub fn compute_abi_info(fn_abi: &mut FnAbi<'_, Ty>) { if !fn_abi.ret.is_ignore() { - classify_ret(cx, &mut fn_abi.ret, &mut offset); + classify_ret(&mut fn_abi.ret); } for arg in &mut fn_abi.args { if arg.is_ignore() { continue; } - classify_arg(cx, arg, &mut offset); + classify_arg(arg); } } From 543fe5b413bee7f7637c028b4b67b5daa97e1b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 5 Nov 2019 16:02:12 -0800 Subject: [PATCH 05/12] Fix libfmt_macros tests --- src/libfmt_macros/lib.rs | 23 ++++++++++---------- src/libfmt_macros/tests.rs | 43 +++++++++++++++++++++++++------------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index 3896612ea0388..24b19028ac117 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -35,7 +35,7 @@ impl InnerOffset { /// A piece is a portion of the format string which represents the next part /// to emit. These are emitted as a stream by the `Parser` class. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum Piece<'a> { /// A literal string which should directly be emitted String(&'a str), @@ -45,7 +45,7 @@ pub enum Piece<'a> { } /// Representation of an argument specification. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct Argument<'a> { /// Where to find this argument pub position: Position, @@ -54,7 +54,7 @@ pub struct Argument<'a> { } /// Specification for the formatting of an argument in the format string. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct FormatSpec<'a> { /// Optionally specified character to fill alignment with. pub fill: Option, @@ -79,7 +79,7 @@ pub struct FormatSpec<'a> { } /// Enum describing where an argument for a format can be located. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum Position { /// The argument is implied to be located at an index ArgumentImplicitlyIs(usize), @@ -99,7 +99,7 @@ impl Position { } /// Enum of alignments which are supported. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum Alignment { /// The value will be aligned to the left. AlignLeft, @@ -113,7 +113,7 @@ pub enum Alignment { /// Various flags which can be applied to format strings. The meaning of these /// flags is defined by the formatters themselves. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum Flag { /// A `+` will be used to denote positive numbers. FlagSignPlus, @@ -133,7 +133,7 @@ pub enum Flag { /// A count is used for the precision and width parameters of an integer, and /// can reference either an argument or a literal integer. -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum Count { /// The count is specified explicitly. CountIs(usize), @@ -572,10 +572,11 @@ impl<'a> Parser<'a> { } else { spec.ty = self.word(); let ty_span_end = self.cur.peek().map(|(pos, _)| *pos); - let this = self; - spec.ty_span = ty_span_start - .and_then(|s| ty_span_end.map(|e| (s, e))) - .map(|(start, end)| this.to_span_index(start).to(this.to_span_index(end))); + if !spec.ty.is_empty() { + spec.ty_span = ty_span_start + .and_then(|s| ty_span_end.map(|e| (s, e))) + .map(|(start, end)| self.to_span_index(start).to(self.to_span_index(end))); + } } spec } diff --git a/src/libfmt_macros/tests.rs b/src/libfmt_macros/tests.rs index e2ddb8810e90a..81359033eda29 100644 --- a/src/libfmt_macros/tests.rs +++ b/src/libfmt_macros/tests.rs @@ -2,7 +2,7 @@ use super::*; fn same(fmt: &'static str, p: &[Piece<'static>]) { let parser = Parser::new(fmt, None, vec![], false); - assert!(parser.collect::>>() == p); + assert_eq!(parser.collect::>>(), p); } fn fmtdflt() -> FormatSpec<'static> { @@ -15,6 +15,7 @@ fn fmtdflt() -> FormatSpec<'static> { precision_span: None, width_span: None, ty: "", + ty_span: None, }; } @@ -82,7 +83,7 @@ fn format_position_nothing_else() { #[test] fn format_type() { same( - "{3:a}", + "{3:x}", &[NextArgument(Argument { position: ArgumentIs(3), format: FormatSpec { @@ -93,7 +94,8 @@ fn format_type() { width: CountImplied, precision_span: None, width_span: None, - ty: "a", + ty: "x", + ty_span: None, }, })]); } @@ -112,6 +114,7 @@ fn format_align_fill() { precision_span: None, width_span: None, ty: "", + ty_span: None, }, })]); same( @@ -127,6 +130,7 @@ fn format_align_fill() { precision_span: None, width_span: None, ty: "", + ty_span: None, }, })]); same( @@ -142,6 +146,7 @@ fn format_align_fill() { precision_span: None, width_span: None, ty: "abcd", + ty_span: Some(InnerSpan::new(6, 10)), }, })]); } @@ -150,7 +155,7 @@ fn format_counts() { use syntax_pos::{GLOBALS, Globals, edition}; GLOBALS.set(&Globals::new(edition::DEFAULT_EDITION), || { same( - "{:10s}", + "{:10x}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(0), format: FormatSpec { @@ -161,11 +166,12 @@ fn format_counts() { width: CountIs(10), precision_span: None, width_span: None, - ty: "s", + ty: "x", + ty_span: None, }, })]); same( - "{:10$.10s}", + "{:10$.10x}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(0), format: FormatSpec { @@ -176,11 +182,12 @@ fn format_counts() { width: CountIsParam(10), precision_span: None, width_span: Some(InnerSpan::new(3, 6)), - ty: "s", + ty: "x", + ty_span: None, }, })]); same( - "{:.*s}", + "{:.*x}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(1), format: FormatSpec { @@ -191,11 +198,12 @@ fn format_counts() { width: CountImplied, precision_span: Some(InnerSpan::new(3, 5)), width_span: None, - ty: "s", + ty: "x", + ty_span: None, }, })]); same( - "{:.10$s}", + "{:.10$x}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(0), format: FormatSpec { @@ -206,11 +214,12 @@ fn format_counts() { width: CountImplied, precision_span: Some(InnerSpan::new(3, 7)), width_span: None, - ty: "s", + ty: "x", + ty_span: None, }, })]); same( - "{:a$.b$s}", + "{:a$.b$?}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(0), format: FormatSpec { @@ -221,7 +230,8 @@ fn format_counts() { width: CountIsName(Symbol::intern("a")), precision_span: None, width_span: None, - ty: "s", + ty: "?", + ty_span: None, }, })]); }); @@ -241,6 +251,7 @@ fn format_flags() { precision_span: None, width_span: None, ty: "", + ty_span: None, }, })]); same( @@ -256,13 +267,14 @@ fn format_flags() { precision_span: None, width_span: None, ty: "", + ty_span: None, }, })]); } #[test] fn format_mixture() { same( - "abcd {3:a} efg", + "abcd {3:x} efg", &[ String("abcd "), NextArgument(Argument { @@ -275,7 +287,8 @@ fn format_mixture() { width: CountImplied, precision_span: None, width_span: None, - ty: "a", + ty: "x", + ty_span: None, }, }), String(" efg"), From 446a43da0ee23b3a69e214f030d6d7dada345296 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Tue, 5 Nov 2019 17:56:37 -0800 Subject: [PATCH 06/12] Stabilize @file command line arguments Issue https://github.com/rust-lang/rust/issues/63576 --- src/librustc_driver/args.rs | 12 +----------- src/librustc_driver/lib.rs | 6 ------ 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/librustc_driver/args.rs b/src/librustc_driver/args.rs index 0906d358badd4..339a10f914044 100644 --- a/src/librustc_driver/args.rs +++ b/src/librustc_driver/args.rs @@ -3,22 +3,12 @@ use std::fmt; use std::fs; use std::io; use std::str; -use std::sync::atomic::{AtomicBool, Ordering}; - -static USED_ARGSFILE_FEATURE: AtomicBool = AtomicBool::new(false); - -pub fn used_unstable_argsfile() -> bool { - USED_ARGSFILE_FEATURE.load(Ordering::Relaxed) -} pub fn arg_expand(arg: String) -> Result, Error> { if arg.starts_with("@") { let path = &arg[1..]; let file = match fs::read_to_string(path) { - Ok(file) => { - USED_ARGSFILE_FEATURE.store(true, Ordering::Relaxed); - file - } + Ok(file) => file, Err(ref err) if err.kind() == io::ErrorKind::InvalidData => { return Err(Error::Utf8Error(Some(path.to_string()))); } diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 6e8bc11162f66..8b531c5d88f80 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -1043,12 +1043,6 @@ pub fn handle_options(args: &[String]) -> Option { // (unstable option being used on stable) nightly_options::check_nightly_options(&matches, &config::rustc_optgroups()); - // Late check to see if @file was used without unstable options enabled - if crate::args::used_unstable_argsfile() && !nightly_options::is_unstable_enabled(&matches) { - early_error(ErrorOutputType::default(), - "@path is unstable - use -Z unstable-options to enable its use"); - } - if matches.opt_present("h") || matches.opt_present("help") { // Only show unstable options in --help if we accept unstable options. usage(matches.opt_present("verbose"), nightly_options::is_unstable_enabled(&matches)); From 3db1005c9d5731915dc06652fde03902b75ad65a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 8 Nov 2019 15:58:25 +0000 Subject: [PATCH 07/12] add link to unstable book for asm! macro --- src/libcore/macros.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index 131fb52e2d22b..726d187d2e981 100644 --- a/src/libcore/macros.rs +++ b/src/libcore/macros.rs @@ -1274,6 +1274,10 @@ pub(crate) mod builtin { } /// Inline assembly. + /// + /// Read the [unstable book] for the usage. + /// + /// [unstable book]: ../unstable-book/library-features/asm.html #[unstable(feature = "asm", issue = "29722", reason = "inline assembly is not stable enough for use and is subject to change")] #[rustc_builtin_macro] From 3783ef6dbe5aa1bb322e11ac33bfc599492d3280 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 28 Oct 2019 12:17:36 -0700 Subject: [PATCH 08/12] Stop returning promotables from `mir_const_qualif` --- src/librustc/query/mod.rs | 2 +- src/librustc/ty/query/mod.rs | 1 - .../rmeta/decoder/cstore_impl.rs | 5 +- src/librustc_metadata/rmeta/encoder.rs | 6 +- .../transform/check_consts/qualifs.rs | 2 +- src/librustc_mir/transform/promote_consts.rs | 2 +- src/librustc_mir/transform/qualify_consts.rs | 64 +++---------------- 7 files changed, 16 insertions(+), 66 deletions(-) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 12ae2c3201547..bd7b77b0abb17 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -93,7 +93,7 @@ rustc_queries! { /// Maps DefId's that have an associated `mir::Body` to the result /// of the MIR qualify_consts pass. The actual meaning of /// the value isn't known except to the pass itself. - query mir_const_qualif(key: DefId) -> (u8, &'tcx BitSet) { + query mir_const_qualif(key: DefId) -> u8 { desc { |tcx| "const checking `{}`", tcx.def_path_str(key) } cache_on_disk_if { key.is_local() } } diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 9b15ad560b5d2..0615004125b3c 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -42,7 +42,6 @@ use crate::util::common::ErrorReported; use crate::util::profiling::ProfileCategory::*; use rustc_data_structures::svh::Svh; -use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_data_structures::fx::{FxIndexMap, FxHashMap, FxHashSet}; use rustc_data_structures::stable_hasher::StableVec; diff --git a/src/librustc_metadata/rmeta/decoder/cstore_impl.rs b/src/librustc_metadata/rmeta/decoder/cstore_impl.rs index 6eacfc28de2db..1118162bdeb02 100644 --- a/src/librustc_metadata/rmeta/decoder/cstore_impl.rs +++ b/src/librustc_metadata/rmeta/decoder/cstore_impl.rs @@ -32,7 +32,6 @@ use syntax::parse::parser::emit_unclosed_delims; use syntax::source_map::Spanned; use syntax::symbol::Symbol; use syntax_pos::{Span, FileName}; -use rustc_index::bit_set::BitSet; macro_rules! provide { (<$lt:tt> $tcx:ident, $def_id:ident, $other:ident, $cdata:ident, @@ -122,9 +121,7 @@ provide! { <'tcx> tcx, def_id, other, cdata, } optimized_mir => { tcx.arena.alloc(cdata.get_optimized_mir(tcx, def_id.index)) } promoted_mir => { tcx.arena.alloc(cdata.get_promoted_mir(tcx, def_id.index)) } - mir_const_qualif => { - (cdata.mir_const_qualif(def_id.index), tcx.arena.alloc(BitSet::new_empty(0))) - } + mir_const_qualif => { cdata.mir_const_qualif(def_id.index) } fn_sig => { cdata.fn_sig(def_id.index, tcx) } inherent_impls => { cdata.get_inherent_implementations_for_type(tcx, def_id.index) } is_const_fn_raw => { cdata.is_const_fn_raw(def_id.index) } diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index c6677ea3534d0..ad1ab16a41074 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -955,7 +955,7 @@ impl EncodeContext<'tcx> { record!(self.per_def.kind[def_id] <- match impl_item.kind { ty::AssocKind::Const => { if let hir::ImplItemKind::Const(_, body_id) = ast_item.kind { - let mir = self.tcx.at(ast_item.span).mir_const_qualif(def_id).0; + let mir = self.tcx.at(ast_item.span).mir_const_qualif(def_id); EntryKind::AssocConst(container, ConstQualif { mir }, @@ -1089,7 +1089,7 @@ impl EncodeContext<'tcx> { hir::ItemKind::Static(_, hir::MutMutable, _) => EntryKind::MutStatic, hir::ItemKind::Static(_, hir::MutImmutable, _) => EntryKind::ImmStatic, hir::ItemKind::Const(_, body_id) => { - let mir = self.tcx.at(item.span).mir_const_qualif(def_id).0; + let mir = self.tcx.at(item.span).mir_const_qualif(def_id); EntryKind::Const( ConstQualif { mir }, self.encode_rendered_const_for_body(body_id) @@ -1368,7 +1368,7 @@ impl EncodeContext<'tcx> { let id = self.tcx.hir().as_local_hir_id(def_id).unwrap(); let body_id = self.tcx.hir().body_owned_by(id); let const_data = self.encode_rendered_const_for_body(body_id); - let mir = self.tcx.mir_const_qualif(def_id).0; + let mir = self.tcx.mir_const_qualif(def_id); record!(self.per_def.kind[def_id] <- EntryKind::Const(ConstQualif { mir }, const_data)); record!(self.per_def.visibility[def_id] <- ty::Visibility::Public); diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 840ad30301607..496a56790679b 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -123,7 +123,7 @@ pub trait Qualif { if cx.tcx.trait_of_item(def_id).is_some() { Self::in_any_value_of_ty(cx, constant.literal.ty) } else { - let (bits, _) = cx.tcx.at(constant.span).mir_const_qualif(def_id); + let bits = cx.tcx.at(constant.span).mir_const_qualif(def_id); let qualif = QualifSet(bits).contains::(); diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index f5e49e3283406..bea8b499b914c 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -538,7 +538,7 @@ impl<'tcx> Validator<'_, 'tcx> { // is gone - we can always promote constants even if they // fail to pass const-checking, as compilation would've // errored independently and promotion can't change that. - let (bits, _) = self.tcx.at(constant.span).mir_const_qualif(def_id); + let bits = self.tcx.at(constant.span).mir_const_qualif(def_id); if bits == super::qualify_consts::QUALIF_ERROR_BIT { self.tcx.sess.delay_span_bug( constant.span, diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 6a7058f193017..2b3ca9e9c5b5a 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -258,7 +258,7 @@ trait Qualif { if cx.tcx.trait_of_item(def_id).is_some() { Self::in_any_value_of_ty(cx, constant.literal.ty).unwrap_or(false) } else { - let (bits, _) = cx.tcx.at(constant.span).mir_const_qualif(def_id); + let bits = cx.tcx.at(constant.span).mir_const_qualif(def_id); let qualif = PerQualif::decode_from_bits(bits).0[Self::IDX]; @@ -682,7 +682,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } /// Check a whole const, static initializer or const fn. - fn check_const(&mut self) -> (u8, &'tcx BitSet) { + fn check_const(&mut self) -> u8 { use crate::transform::check_consts as new_checker; debug!("const-checking {} {:?}", self.mode, self.def_id); @@ -704,7 +704,6 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { let mut seen_blocks = BitSet::new_empty(body.basic_blocks().len()); let mut bb = START_BLOCK; - let mut has_controlflow_error = false; loop { seen_blocks.insert(bb.index()); @@ -745,7 +744,6 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { bb = target; } _ => { - has_controlflow_error = true; self.not_const(ops::Loop); validator.check_op(ops::Loop); break; @@ -772,51 +770,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } } - // Collect all the temps we need to promote. - let mut promoted_temps = BitSet::new_empty(self.temp_promotion_state.len()); - - // HACK: if parts of the control-flow graph were skipped due to an error, don't try to - // promote anything, since that can cause errors in a `const` if e.g. rvalue static - // promotion is attempted within a loop body. - let unleash_miri = self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you; - let promotion_candidates = if has_controlflow_error && !unleash_miri { - self.tcx.sess.delay_span_bug( - body.span, - "check_const: expected control-flow error(s)", - ); - - vec![] - } else { - promote_consts::validate_candidates( - self.tcx, - self.body, - self.def_id, - &self.temp_promotion_state, - &self.unchecked_promotion_candidates, - ) - }; - - debug!("qualify_const: promotion_candidates={:?}", promotion_candidates); - for candidate in promotion_candidates { - match candidate { - Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => { - if let StatementKind::Assign(box( _, Rvalue::Ref(_, _, place))) - = &self.body[bb].statements[stmt_idx].kind - { - if let PlaceBase::Local(local) = place.base { - promoted_temps.insert(local); - } - } - } - - // Only rvalue-static promotion requires extending the lifetime of the promoted - // local. - Candidate::Argument { .. } | Candidate::Repeat(_) => {} - } - } - - let qualifs = self.qualifs_in_local(RETURN_PLACE); - (qualifs.encode_to_bits(), self.tcx.arena.alloc(promoted_temps)) + self.qualifs_in_local(RETURN_PLACE).encode_to_bits() } } @@ -1346,7 +1300,7 @@ pub fn provide(providers: &mut Providers<'_>) { // in `promote_consts`, see the comment in `validate_operand`. pub(super) const QUALIF_ERROR_BIT: u8 = 1 << 2; -fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> (u8, &BitSet) { +fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> u8 { // N.B., this `borrow()` is guaranteed to be valid (i.e., the value // cannot yet be stolen), because `mir_validated()`, which steals // from `mir_const(), forces this query to execute before @@ -1355,7 +1309,7 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> (u8, &BitSet) { if body.return_ty().references_error() { tcx.sess.delay_span_bug(body.span, "mir_const_qualif: MIR had errors"); - return (QUALIF_ERROR_BIT, tcx.arena.alloc(BitSet::new_empty(0))); + return QUALIF_ERROR_BIT; } Checker::new(tcx, def_id, body, Mode::Const).check_const() @@ -1436,11 +1390,11 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { } else { check_short_circuiting_in_const_local(tcx, body, mode); - let promoted_temps = match mode { - Mode::Const => tcx.mir_const_qualif(def_id).1, - _ => Checker::new(tcx, def_id, body, mode).check_const().1, + match mode { + Mode::Const => tcx.mir_const_qualif(def_id), + _ => Checker::new(tcx, def_id, body, mode).check_const(), }; - remove_drop_and_storage_dead_on_promoted_locals(body, promoted_temps); + remove_drop_and_storage_dead_on_promoted_locals(body, unimplemented!()); } if mode == Mode::Static && !tcx.has_attr(def_id, sym::thread_local) { From 170272b74fdeb1141a23817a4d69b58a084a0cd0 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 28 Oct 2019 15:32:56 -0700 Subject: [PATCH 09/12] Add a `PromoteTemps` pass `remove_storage_dead_and_drop` has been copied to `promote_temps` and now operates on an array of `Candidate`s instead of a bitset. --- src/librustc_mir/transform/promote_consts.rs | 138 ++++++++++++++++++- src/librustc_mir/transform/qualify_consts.rs | 34 ----- 2 files changed, 137 insertions(+), 35 deletions(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index bea8b499b914c..48a58f1d0ee57 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -17,7 +17,7 @@ use rustc::mir::*; use rustc::mir::interpret::ConstValue; use rustc::mir::visit::{PlaceContext, MutatingUseContext, MutVisitor, Visitor}; use rustc::mir::traversal::ReversePostorder; -use rustc::ty::{self, List, TyCtxt}; +use rustc::ty::{self, List, TyCtxt, TypeFoldable}; use rustc::ty::subst::InternalSubsts; use rustc::ty::cast::CastTy; use syntax::ast::LitKind; @@ -25,12 +25,68 @@ use syntax::symbol::sym; use syntax_pos::{Span, DUMMY_SP}; use rustc_index::vec::{IndexVec, Idx}; +use rustc_index::bit_set::HybridBitSet; use rustc_target::spec::abi::Abi; +use std::cell::Cell; use std::{iter, mem, usize}; +use crate::transform::{MirPass, MirSource}; use crate::transform::check_consts::{qualifs, Item, ConstKind, is_lang_panic_fn}; +/// A `MirPass` for promotion. +/// +/// In this case, "promotion" entails the following: +/// - Extract promotable temps in `fn` and `const fn` into their own MIR bodies. +/// - Extend lifetimes in `const` and `static` by removing `Drop` and `StorageDead`. +/// - Emit errors if the requirements of `#[rustc_args_required_const]` are not met. +/// +/// After this pass is run, `promoted_fragments` will hold the MIR body corresponding to each +/// newly created `StaticKind::Promoted`. +#[derive(Default)] +pub struct PromoteTemps<'tcx> { + pub promoted_fragments: Cell>>, +} + +impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> { + fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) { + // There's not really any point in promoting errorful MIR. + // + // This does not include MIR that failed const-checking, which we still try to promote. + if body.return_ty().references_error() { + tcx.sess.delay_span_bug(body.span, "PromoteTemps: MIR had errors"); + return; + } + + if src.promoted.is_some() { + return; + } + + let def_id = src.def_id(); + + let item = Item::new(tcx, def_id, body); + let mut rpo = traversal::reverse_postorder(body); + let (temps, all_candidates) = collect_temps_and_candidates(tcx, body, &mut rpo); + + let promotable_candidates = validate_candidates(tcx, body, def_id, &temps, &all_candidates); + + // For now, lifetime extension is done in `const` and `static`s without creating promoted + // MIR fragments by removing `Drop` and `StorageDead` for each referent. However, this will + // not work inside loops when they are allowed in `const`s. + // + // FIXME: use promoted MIR fragments everywhere? + let promoted_fragments = if should_create_promoted_mir_fragments(item.const_kind) { + promote_candidates(def_id, body, tcx, temps, promotable_candidates) + } else { + // FIXME: promote const array initializers in consts. + remove_drop_and_storage_dead_on_promoted_locals(tcx, body, &promotable_candidates); + IndexVec::new() + }; + + self.promoted_fragments.set(promoted_fragments); + } +} + /// State of a temporary during collection and promotion. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum TempState { @@ -1154,3 +1210,83 @@ crate fn should_suggest_const_in_array_repeat_expressions_attribute<'tcx>( should_promote={:?} feature_flag={:?}", mir_def_id, should_promote, feature_flag); should_promote && !feature_flag } + +fn should_create_promoted_mir_fragments(const_kind: Option) -> bool { + match const_kind { + Some(ConstKind::ConstFn) | None => true, + Some(ConstKind::Const) | Some(ConstKind::Static) | Some(ConstKind::StaticMut) => false, + } +} + +/// In `const` and `static` everything without `StorageDead` +/// is `'static`, we don't have to create promoted MIR fragments, +/// just remove `Drop` and `StorageDead` on "promoted" locals. +fn remove_drop_and_storage_dead_on_promoted_locals( + tcx: TyCtxt<'tcx>, + body: &mut Body<'tcx>, + promotable_candidates: &[Candidate], +) { + debug!("run_pass: promotable_candidates={:?}", promotable_candidates); + + // Removing `StorageDead` will cause errors for temps declared inside a loop body. For now we + // simply skip promotion if a loop exists, since loops are not yet allowed in a `const`. + // + // FIXME: Just create MIR fragments for `const`s instead of using this hackish approach? + if body.is_cfg_cyclic() { + tcx.sess.delay_span_bug(body.span, "Control-flow cycle detected in `const`"); + return; + } + + // The underlying local for promotion contexts like `&temp` and `&(temp.proj)`. + let mut requires_lifetime_extension = HybridBitSet::new_empty(body.local_decls.len()); + + promotable_candidates + .iter() + .filter_map(|c| { + match c { + Candidate::Ref(loc) => Some(loc), + Candidate::Repeat(_) | Candidate::Argument { .. } => None, + } + }) + .map(|&Location { block, statement_index }| { + // FIXME: store the `Local` for each `Candidate` when it is created. + let place = match &body[block].statements[statement_index].kind { + StatementKind::Assign(box ( _, Rvalue::Ref(_, _, place))) => place, + _ => bug!("`Candidate::Ref` without corresponding assignment"), + }; + + match place.base { + PlaceBase::Local(local) => local, + PlaceBase::Static(_) => bug!("`Candidate::Ref` for a non-local"), + } + }) + .for_each(|local| { + requires_lifetime_extension.insert(local); + }); + + // Remove `Drop` terminators and `StorageDead` statements for all promotable temps that require + // lifetime extension. + for block in body.basic_blocks_mut() { + block.statements.retain(|statement| { + match statement.kind { + StatementKind::StorageDead(index) => !requires_lifetime_extension.contains(index), + _ => true + } + }); + let terminator = block.terminator_mut(); + match &terminator.kind { + TerminatorKind::Drop { + location, + target, + .. + } => { + if let Some(index) = location.as_local() { + if requires_lifetime_extension.contains(index) { + terminator.kind = TerminatorKind::Goto { target: *target }; + } + } + } + _ => {} + } + } +} diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 2b3ca9e9c5b5a..aebc9d23492d7 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1455,40 +1455,6 @@ fn check_short_circuiting_in_const_local(tcx: TyCtxt<'_>, body: &mut Body<'tcx>, } } -/// In `const` and `static` everything without `StorageDead` -/// is `'static`, we don't have to create promoted MIR fragments, -/// just remove `Drop` and `StorageDead` on "promoted" locals. -fn remove_drop_and_storage_dead_on_promoted_locals( - body: &mut Body<'tcx>, - promoted_temps: &BitSet, -) { - debug!("run_pass: promoted_temps={:?}", promoted_temps); - - for block in body.basic_blocks_mut() { - block.statements.retain(|statement| { - match statement.kind { - StatementKind::StorageDead(index) => !promoted_temps.contains(index), - _ => true - } - }); - let terminator = block.terminator_mut(); - match &terminator.kind { - TerminatorKind::Drop { - location, - target, - .. - } => { - if let Some(index) = location.as_local() { - if promoted_temps.contains(index) { - terminator.kind = TerminatorKind::Goto { target: *target }; - } - } - } - _ => {} - } - } -} - fn check_static_is_sync(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, hir_id: HirId) { let ty = body.return_ty(); tcx.infer_ctxt().enter(|infcx| { From b316384e145d4b21b4e882e7abd29fb41d46e33c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 6 Nov 2019 14:23:35 -0800 Subject: [PATCH 10/12] Use new `PromoteTemps` for promotion --- src/librustc_mir/transform/mod.rs | 7 +-- src/librustc_mir/transform/qualify_consts.rs | 56 ++++++-------------- src/test/mir-opt/match_false_edges.rs | 12 ++--- 3 files changed, 27 insertions(+), 48 deletions(-) diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index dbe6c7845926d..6d6d6bea2a0ea 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -210,13 +210,14 @@ fn mir_validated( } let mut body = tcx.mir_const(def_id).steal(); - let qualify_and_promote_pass = qualify_consts::QualifyAndPromoteConstants::default(); + let promote_pass = promote_consts::PromoteTemps::default(); run_passes(tcx, &mut body, InstanceDef::Item(def_id), None, MirPhase::Validated, &[ // What we need to run borrowck etc. - &qualify_and_promote_pass, + &qualify_consts::QualifyAndPromoteConstants::default(), + &promote_pass, &simplify::SimplifyCfg::new("qualify-consts"), ]); - let promoted = qualify_and_promote_pass.promoted.into_inner(); + let promoted = promote_pass.promoted_fragments.into_inner(); (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted)) } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index aebc9d23492d7..01111af7c3e03 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1345,48 +1345,27 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { let mode = determine_mode(tcx, hir_id, def_id); debug!("run_pass: mode={:?}", mode); - if let Mode::NonConstFn | Mode::ConstFn = mode { + if let Mode::NonConstFn = mode { + // No need to const-check a non-const `fn` now that we don't do promotion here. + return; + } else if let Mode::ConstFn = mode { let mut checker = Checker::new(tcx, def_id, body, mode); - if let Mode::ConstFn = mode { - let use_min_const_fn_checks = - !tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you && - tcx.is_min_const_fn(def_id); - if use_min_const_fn_checks { - // Enforce `min_const_fn` for stable `const fn`s. - use super::qualify_min_const_fn::is_min_const_fn; - if let Err((span, err)) = is_min_const_fn(tcx, def_id, body) { - error_min_const_fn_violation(tcx, span, err); - return; - } - - // `check_const` should not produce any errors, but better safe than sorry - // FIXME(#53819) - // NOTE(eddyb) `check_const` is actually needed for promotion inside - // `min_const_fn` functions. - } - - // Enforce a constant-like CFG for `const fn`. - checker.check_const(); - } else { - while let Some((bb, data)) = checker.rpo.next() { - checker.visit_basic_block_data(bb, data); + let use_min_const_fn_checks = + !tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you && + tcx.is_min_const_fn(def_id); + if use_min_const_fn_checks { + // Enforce `min_const_fn` for stable `const fn`s. + use super::qualify_min_const_fn::is_min_const_fn; + if let Err((span, err)) = is_min_const_fn(tcx, def_id, body) { + error_min_const_fn_violation(tcx, span, err); + return; } } - // Promote only the promotable candidates. - let temps = checker.temp_promotion_state; - let candidates = promote_consts::validate_candidates( - tcx, - body, - def_id, - &temps, - &checker.unchecked_promotion_candidates, - ); - - // Do the actual promotion, now that we know what's viable. - self.promoted.set( - promote_consts::promote_candidates(def_id, body, tcx, temps, candidates) - ); + // `check_const` should not produce any errors, but better safe than sorry + // FIXME(#53819) + // Enforce a constant-like CFG for `const fn`. + checker.check_const(); } else { check_short_circuiting_in_const_local(tcx, body, mode); @@ -1394,7 +1373,6 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { Mode::Const => tcx.mir_const_qualif(def_id), _ => Checker::new(tcx, def_id, body, mode).check_const(), }; - remove_drop_and_storage_dead_on_promoted_locals(body, unimplemented!()); } if mode == Mode::Static && !tcx.has_attr(def_id, sym::thread_local) { diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index b275c06e05cd3..648856b5523d3 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -39,7 +39,7 @@ fn main() { // END RUST SOURCE // -// START rustc.full_tested_match.QualifyAndPromoteConstants.after.mir +// START rustc.full_tested_match.PromoteTemps.after.mir // bb0: { // ... // _2 = std::option::Option::::Some(const 42i32,); @@ -108,9 +108,9 @@ fn main() { // _0 = (); // return; // } -// END rustc.full_tested_match.QualifyAndPromoteConstants.after.mir +// END rustc.full_tested_match.PromoteTemps.after.mir // -// START rustc.full_tested_match2.QualifyAndPromoteConstants.before.mir +// START rustc.full_tested_match2.PromoteTemps.before.mir // bb0: { // ... // _2 = std::option::Option::::Some(const 42i32,); @@ -179,9 +179,9 @@ fn main() { // _0 = (); // return; // } -// END rustc.full_tested_match2.QualifyAndPromoteConstants.before.mir +// END rustc.full_tested_match2.PromoteTemps.before.mir // -// START rustc.main.QualifyAndPromoteConstants.before.mir +// START rustc.main.PromoteTemps.before.mir // bb0: { // ... // _2 = std::option::Option::::Some(const 1i32,); @@ -276,4 +276,4 @@ fn main() { // _0 = (); // return; // } -// END rustc.main.QualifyAndPromoteConstants.before.mir +// END rustc.main.PromoteTemps.before.mir From b13c2c330d623d1254a0c02e6447e3717470728c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 28 Oct 2019 15:21:53 -0700 Subject: [PATCH 11/12] Bless tests now that we do promotion if `min_const_fn` fails We bailed out of `QualifyAndPromoteConsts` immediately if the `min_const_fn` checks failed, which sometimes resulted in additional, spurious errors since promotion was skipped. We now do promotion in a completely separate pass, so this is no longer an issue. --- .../ui/consts/min_const_fn/min_const_fn.rs | 1 - .../consts/min_const_fn/min_const_fn.stderr | 19 +++++-------------- .../consts/min_const_fn/min_const_fn_dyn.rs | 1 - .../min_const_fn/min_const_fn_dyn.stderr | 14 ++------------ 4 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/test/ui/consts/min_const_fn/min_const_fn.rs b/src/test/ui/consts/min_const_fn/min_const_fn.rs index d0f63b148ff2b..db68a05905a14 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn.rs +++ b/src/test/ui/consts/min_const_fn/min_const_fn.rs @@ -136,7 +136,6 @@ const fn no_rpit() -> impl std::fmt::Debug {} //~ ERROR `impl Trait` in const fn const fn no_dyn_trait(_x: &dyn std::fmt::Debug) {} //~ ERROR trait bounds other than `Sized` const fn no_dyn_trait_ret() -> &'static dyn std::fmt::Debug { &() } //~^ ERROR trait bounds other than `Sized` -//~| ERROR cannot return reference to temporary value const fn no_unsafe() { unsafe {} } diff --git a/src/test/ui/consts/min_const_fn/min_const_fn.stderr b/src/test/ui/consts/min_const_fn/min_const_fn.stderr index 3158b6284db94..64b2ce83da2f8 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn.stderr +++ b/src/test/ui/consts/min_const_fn/min_const_fn.stderr @@ -286,17 +286,8 @@ LL | const fn no_dyn_trait_ret() -> &'static dyn std::fmt::Debug { &() } = note: for more information, see issue https://github.com/rust-lang/rust/issues/57563 = help: add `#![feature(const_fn)]` to the crate attributes to enable -error[E0515]: cannot return reference to temporary value - --> $DIR/min_const_fn.rs:137:63 - | -LL | const fn no_dyn_trait_ret() -> &'static dyn std::fmt::Debug { &() } - | ^-- - | || - | |temporary value created here - | returns a reference to data owned by the current function - error[E0723]: trait bounds other than `Sized` on const fn parameters are unstable - --> $DIR/min_const_fn.rs:143:41 + --> $DIR/min_const_fn.rs:142:41 | LL | const fn really_no_traits_i_mean_it() { (&() as &dyn std::fmt::Debug, ()).1 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -305,7 +296,7 @@ LL | const fn really_no_traits_i_mean_it() { (&() as &dyn std::fmt::Debug, ()).1 = help: add `#![feature(const_fn)]` to the crate attributes to enable error[E0723]: function pointers in const fn are unstable - --> $DIR/min_const_fn.rs:146:21 + --> $DIR/min_const_fn.rs:145:21 | LL | const fn no_fn_ptrs(_x: fn()) {} | ^^ @@ -314,7 +305,7 @@ LL | const fn no_fn_ptrs(_x: fn()) {} = help: add `#![feature(const_fn)]` to the crate attributes to enable error[E0723]: function pointers in const fn are unstable - --> $DIR/min_const_fn.rs:148:27 + --> $DIR/min_const_fn.rs:147:27 | LL | const fn no_fn_ptrs2() -> fn() { fn foo() {} foo } | ^^^^ @@ -322,7 +313,7 @@ LL | const fn no_fn_ptrs2() -> fn() { fn foo() {} foo } = note: for more information, see issue https://github.com/rust-lang/rust/issues/57563 = help: add `#![feature(const_fn)]` to the crate attributes to enable -error: aborting due to 37 previous errors +error: aborting due to 36 previous errors -Some errors have detailed explanations: E0493, E0515, E0723. +Some errors have detailed explanations: E0493, E0723. For more information about an error, try `rustc --explain E0493`. diff --git a/src/test/ui/consts/min_const_fn/min_const_fn_dyn.rs b/src/test/ui/consts/min_const_fn/min_const_fn_dyn.rs index 3833510c0b3b5..6ca1e59b3af10 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn_dyn.rs +++ b/src/test/ui/consts/min_const_fn/min_const_fn_dyn.rs @@ -11,6 +11,5 @@ const fn no_inner_dyn_trait2(x: Hide) { } const fn no_inner_dyn_trait_ret() -> Hide { Hide(HasDyn { field: &0 }) } //~^ ERROR trait bounds other than `Sized` -//~| ERROR temporary value dropped while borrowed fn main() {} diff --git a/src/test/ui/consts/min_const_fn/min_const_fn_dyn.stderr b/src/test/ui/consts/min_const_fn/min_const_fn_dyn.stderr index 0ea950d678f87..e20b4f9dcb471 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn_dyn.stderr +++ b/src/test/ui/consts/min_const_fn/min_const_fn_dyn.stderr @@ -16,16 +16,6 @@ LL | const fn no_inner_dyn_trait_ret() -> Hide { Hide(HasDyn { field: &0 }) } = note: for more information, see issue https://github.com/rust-lang/rust/issues/57563 = help: add `#![feature(const_fn)]` to the crate attributes to enable -error[E0716]: temporary value dropped while borrowed - --> $DIR/min_const_fn_dyn.rs:12:67 - | -LL | const fn no_inner_dyn_trait_ret() -> Hide { Hide(HasDyn { field: &0 }) } - | -^ - temporary value is freed at the end of this statement - | || - | |creates a temporary which is freed while still in use - | cast requires that borrow lasts for `'static` - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors have detailed explanations: E0716, E0723. -For more information about an error, try `rustc --explain E0716`. +For more information about this error, try `rustc --explain E0723`. From a3b03690c07f72ed802e4e77501960482c7c2661 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 6 Nov 2019 15:00:29 -0800 Subject: [PATCH 12/12] Clean up dead code in `qualify_consts` We don't do promotion here anymore, so `Checker` will never even visit the body of a non-const `fn`. --- src/librustc_mir/transform/qualify_consts.rs | 21 -------------------- 1 file changed, 21 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 01111af7c3e03..5ad5363768d34 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -15,7 +15,6 @@ use rustc::ty::cast::CastTy; use rustc::ty::query::Providers; use rustc::mir::*; use rustc::mir::interpret::ConstValue; -use rustc::mir::traversal::ReversePostorder; use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext}; use rustc::middle::lang_items; use rustc::session::config::nightly_options; @@ -31,7 +30,6 @@ use std::usize; use rustc::hir::HirId; use crate::transform::{MirPass, MirSource}; -use super::promote_consts::{self, Candidate, TempState}; use crate::transform::check_consts::ops::{self, NonConstOp}; /// What kind of item we are in. @@ -477,10 +475,6 @@ struct Checker<'a, 'tcx> { span: Span, def_id: DefId, - rpo: ReversePostorder<'a, 'tcx>, - - temp_promotion_state: IndexVec, - unchecked_promotion_candidates: Vec, /// If `true`, do not emit errors to the user, merely collect them in `errors`. suppress_errors: bool, @@ -509,10 +503,6 @@ impl Deref for Checker<'a, 'tcx> { impl<'a, 'tcx> Checker<'a, 'tcx> { fn new(tcx: TyCtxt<'tcx>, def_id: DefId, body: &'a Body<'tcx>, mode: Mode) -> Self { assert!(def_id.is_local()); - let mut rpo = traversal::reverse_postorder(body); - let (temps, unchecked_promotion_candidates) = - promote_consts::collect_temps_and_candidates(tcx, body, &mut rpo); - rpo.reset(); let param_env = tcx.param_env(def_id); @@ -539,9 +529,6 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { cx, span: body.span, def_id, - rpo, - temp_promotion_state: temps, - unchecked_promotion_candidates, errors: vec![], suppress_errors: false, } @@ -662,14 +649,6 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { let kind = self.body.local_kind(index); debug!("store to {:?} {:?}", kind, index); - // Only handle promotable temps in non-const functions. - if self.mode == Mode::NonConstFn { - if kind != LocalKind::Temp || - !self.temp_promotion_state[index].is_promotable() { - return; - } - } - // this is overly restrictive, because even full assignments do not clear the qualif // While we could special case full assignments, this would be inconsistent with // aggregates where we overwrite all fields via assignments, which would not get