From 498bdc9b42e1de6db051a24be0a4318f075a5562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 15 Mar 2019 12:17:11 +0100 Subject: [PATCH 1/8] Add an AtomicCell abstraction --- Cargo.lock | 1 + src/librustc_data_structures/Cargo.toml | 1 + src/librustc_data_structures/sync.rs | 55 ++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 8957e223b10b5..6a194bd52ab03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3019,6 +3019,7 @@ name = "rustc_data_structures" version = "0.0.0" dependencies = [ "cfg-if 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)", + "crossbeam-utils 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", "ena 0.13.0 (registry+https://github.com/rust-lang/crates.io-index)", "graphviz 0.0.0", "indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/src/librustc_data_structures/Cargo.toml b/src/librustc_data_structures/Cargo.toml index acddb3448ca60..79cbe26e73e83 100644 --- a/src/librustc_data_structures/Cargo.toml +++ b/src/librustc_data_structures/Cargo.toml @@ -18,6 +18,7 @@ lazy_static = "1" serialize = { path = "../libserialize" } graphviz = { path = "../libgraphviz" } cfg-if = "0.1.2" +crossbeam-utils = { version = "0.6.5", features = ["nightly"] } stable_deref_trait = "1.0.0" rayon = { version = "0.2.0", package = "rustc-rayon" } rayon-core = { version = "0.2.0", package = "rustc-rayon-core" } diff --git a/src/librustc_data_structures/sync.rs b/src/librustc_data_structures/sync.rs index 73247c1469efd..3277b85c28144 100644 --- a/src/librustc_data_structures/sync.rs +++ b/src/librustc_data_structures/sync.rs @@ -67,6 +67,51 @@ cfg_if! { use std::ops::Add; use std::panic::{resume_unwind, catch_unwind, AssertUnwindSafe}; + /// This is a single threaded variant of AtomicCell provided by crossbeam. + /// Unlike `Atomic` this is intended for all `Copy` types, + /// but it lacks the explicit ordering arguments. + #[derive(Debug)] + pub struct AtomicCell(Cell); + + impl AtomicCell { + #[inline] + pub fn new(v: T) -> Self { + AtomicCell(Cell::new(v)) + } + + #[inline] + pub fn get_mut(&mut self) -> &mut T { + self.0.get_mut() + } + } + + impl AtomicCell { + #[inline] + pub fn into_inner(self) -> T { + self.0.into_inner() + } + + #[inline] + pub fn load(&self) -> T { + self.0.get() + } + + #[inline] + pub fn store(&self, val: T) { + self.0.set(val) + } + + #[inline] + pub fn swap(&self, val: T) -> T { + self.0.replace(val) + } + } + + /// This is a single threaded variant of `AtomicU64`, `AtomicUsize`, etc. + /// It differs from `AtomicCell` in that it has explicit ordering arguments + /// and is only intended for use with the native atomic types. + /// You should use this type through the `AtomicU64`, `AtomicUsize`, etc, type aliases + /// as it's not intended to be used separately. #[derive(Debug)] pub struct Atomic(Cell); @@ -77,7 +122,8 @@ cfg_if! { } } - impl Atomic { + impl Atomic { + #[inline] pub fn into_inner(self) -> T { self.0.into_inner() } @@ -92,10 +138,14 @@ cfg_if! { self.0.set(val) } + #[inline] pub fn swap(&self, val: T, _: Ordering) -> T { self.0.replace(val) } + } + impl Atomic { + #[inline] pub fn compare_exchange(&self, current: T, new: T, @@ -113,6 +163,7 @@ cfg_if! { } impl + Copy> Atomic { + #[inline] pub fn fetch_add(&self, val: T, _: Ordering) -> T { let old = self.0.get(); self.0.set(old + val); @@ -271,6 +322,8 @@ cfg_if! { pub use std::sync::atomic::{AtomicBool, AtomicUsize, AtomicU32, AtomicU64}; + pub use crossbeam_utils::atomic::AtomicCell; + pub use std::sync::Arc as Lrc; pub use std::sync::Weak as Weak; From e1c7747cf06bc063a6586c1eab898703f61899d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 11 Jul 2019 16:54:33 -0700 Subject: [PATCH 2/8] Handle errors during error recovery gracefully --- src/libsyntax/parse/parser.rs | 11 +++++++---- src/test/ui/parser/issue-62546.rs | 3 +++ src/test/ui/parser/issue-62546.stderr | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/parser/issue-62546.rs create mode 100644 src/test/ui/parser/issue-62546.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 83dbff6b2d574..6a26c4ad59b3a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -7408,10 +7408,13 @@ impl<'a> Parser<'a> { } else if self.look_ahead(1, |t| *t == token::OpenDelim(token::Paren)) { let ident = self.parse_ident().unwrap(); self.bump(); // `(` - let kw_name = if let Ok(Some(_)) = self.parse_self_arg_with_attrs() { - "method" - } else { - "function" + let kw_name = match self.parse_self_arg_with_attrs() { + Ok(Some(_)) => "method", + Ok(None) => "function", + Err(mut err) => { + err.cancel(); + "function" + } }; self.consume_block(token::Paren); let (kw, kw_name, ambiguous) = if self.check(&token::RArrow) { diff --git a/src/test/ui/parser/issue-62546.rs b/src/test/ui/parser/issue-62546.rs new file mode 100644 index 0000000000000..75b95e7407302 --- /dev/null +++ b/src/test/ui/parser/issue-62546.rs @@ -0,0 +1,3 @@ +pub t(# +//~^ ERROR missing `fn` or `struct` for function or struct definition +//~ ERROR this file contains an un-closed delimiter diff --git a/src/test/ui/parser/issue-62546.stderr b/src/test/ui/parser/issue-62546.stderr new file mode 100644 index 0000000000000..631aac9550585 --- /dev/null +++ b/src/test/ui/parser/issue-62546.stderr @@ -0,0 +1,17 @@ +error: this file contains an un-closed delimiter + --> $DIR/issue-62546.rs:3:53 + | +LL | pub t(# + | - un-closed delimiter +LL | +LL | + | ^ + +error: missing `fn` or `struct` for function or struct definition + --> $DIR/issue-62546.rs:1:4 + | +LL | pub t(# + | ---^- help: if you meant to call a macro, try: `t!` + +error: aborting due to 2 previous errors + From 8f171c49ceaf16b1a81125ad9788c9dae70d2111 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 11 Jul 2019 13:27:41 +0200 Subject: [PATCH 3/8] Replace `struct_tail` and `struct_lockstep_tails` with variants handling normalization. The old struct tail functions did not deal with `::A` and `impl Trait`, at least not explicitly. (We didn't notice this bug before because it is only exposed when the tail (post deep normalization) is not `Sized`, so it was a rare case to deal with.) For post type-checking (i.e. during codegen), there is now `struct_tail_erasing_lifetimes` and `struct_lockstep_tails_erasing_lifetimes`, which each take an additional `ParamEnv` argument to drive normalization. For pre type-checking cases where normalization is not needed, there is `struct_tail_without_normalization`. (Currently, the only instance of this is `Expectation::rvalue_hint`.) All of these new entrypoints work by calling out to common helper routines. The helpers are parameterized over a closure that handles the normalization. --- src/librustc/ty/layout.rs | 6 +- src/librustc/ty/util.rs | 97 ++++++++++++++++++++-- src/librustc_codegen_ssa/base.rs | 3 +- src/librustc_codegen_ssa/traits/type_.rs | 5 +- src/librustc_mir/interpret/cast.rs | 3 +- src/librustc_mir/interpret/intern.rs | 4 +- src/librustc_mir/interpret/validity.rs | 3 +- src/librustc_mir/monomorphize/collector.rs | 7 +- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/check/wfcheck.rs | 3 +- 10 files changed, 111 insertions(+), 22 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 4af26e19b370c..4ed52a1e96638 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -543,7 +543,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { return Ok(tcx.intern_layout(LayoutDetails::scalar(self, data_ptr))); } - let unsized_part = tcx.struct_tail(pointee); + let unsized_part = tcx.struct_tail_erasing_lifetimes(pointee, param_env); let metadata = match unsized_part.sty { ty::Foreign(..) => { return Ok(tcx.intern_layout(LayoutDetails::scalar(self, data_ptr))); @@ -1664,7 +1664,7 @@ impl<'tcx> SizeSkeleton<'tcx> { ty::Ref(_, pointee, _) | ty::RawPtr(ty::TypeAndMut { ty: pointee, .. }) => { let non_zero = !ty.is_unsafe_ptr(); - let tail = tcx.struct_tail(pointee); + let tail = tcx.struct_tail_erasing_lifetimes(pointee, param_env); match tail.sty { ty::Param(_) | ty::Projection(_) => { debug_assert!(tail.has_param_types() || tail.has_self_ty()); @@ -2015,7 +2015,7 @@ where })); } - match tcx.struct_tail(pointee).sty { + match tcx.struct_tail_erasing_lifetimes(pointee, cx.param_env()).sty { ty::Slice(_) | ty::Str => tcx.types.usize, ty::Dynamic(_, _) => { diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index a3b99f143d055..871aeb78af517 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -258,10 +258,42 @@ impl<'tcx> TyCtxt<'tcx> { false } - /// Returns the deeply last field of nested structures, or the same type, - /// if not a structure at all. Corresponds to the only possible unsized - /// field, and its type can be used to determine unsizing strategy. - pub fn struct_tail(self, mut ty: Ty<'tcx>) -> Ty<'tcx> { + /// Attempts to returns the deeply last field of nested structures, but + /// does not apply any normalization in its search. Returns the same type + /// if input `ty` is not a structure at all. + pub fn struct_tail_without_normalization(self, ty: Ty<'tcx>) -> Ty<'tcx> + { + let tcx = self; + tcx.struct_tail_with_normalize(ty, |ty| ty) + } + + /// Returns the deeply last field of nested structures, or the same type if + /// not a structure at all. Corresponds to the only possible unsized field, + /// and its type can be used to determine unsizing strategy. + pub fn struct_tail_erasing_lifetimes(self, + ty: Ty<'tcx>, + param_env: ty::ParamEnv<'tcx>) + -> Ty<'tcx> + { + let tcx = self; + tcx.struct_tail_with_normalize(ty, |ty| tcx.normalize_erasing_regions(param_env, ty)) + } + + /// Returns the deeply last field of nested structures, or the same type if + /// not a structure at all. Corresponds to the only possible unsized field, + /// and its type can be used to determine unsizing strategy. + /// + /// This is parameterized over the normalization strategy (i.e. how to + /// handle `::Assoc` and `impl Trait`); pass the identity + /// function to indicate no normalization should take place. + /// + /// See also `struct_tail_erasing_lifetimes`, which is what callers running + /// after type checking should use. + pub fn struct_tail_with_normalize(self, + mut ty: Ty<'tcx>, + normalize: impl Fn(Ty<'tcx>) -> Ty<'tcx>) + -> Ty<'tcx> + { loop { match ty.sty { ty::Adt(def, substs) => { @@ -282,6 +314,15 @@ impl<'tcx> TyCtxt<'tcx> { } } + ty::Projection(_) | ty::Opaque(..) => { + let normalized = normalize(ty); + if ty == normalized { + return ty; + } else { + ty = normalized; + } + } + _ => { break; } @@ -295,10 +336,34 @@ impl<'tcx> TyCtxt<'tcx> { /// structure definitions. /// For `(Foo>, Foo)`, the result will be `(Foo, Trait)`, /// whereas struct_tail produces `T`, and `Trait`, respectively. - pub fn struct_lockstep_tails(self, - source: Ty<'tcx>, - target: Ty<'tcx>) - -> (Ty<'tcx>, Ty<'tcx>) { + /// + /// Must only be called after type-checking is complete; otherwise + /// normalization attempt may cause compiler bugs. + pub fn struct_lockstep_tails_erasing_lifetimes(self, + source: Ty<'tcx>, + target: Ty<'tcx>, + param_env: ty::ParamEnv<'tcx>) + -> (Ty<'tcx>, Ty<'tcx>) + { + let tcx = self; + tcx.struct_lockstep_tails_with_normalize( + source, target, |ty| tcx.normalize_erasing_regions(param_env, ty)) + } + + /// Same as applying struct_tail on `source` and `target`, but only + /// keeps going as long as the two types are instances of the same + /// structure definitions. + /// For `(Foo>, Foo)`, the result will be `(Foo, Trait)`, + /// whereas struct_tail produces `T`, and `Trait`, respectively. + /// + /// See also struct_lockstep_tails_erasing_lifetimes, which + /// is what callers running after type checking should use. + pub fn struct_lockstep_tails_with_normalize(self, + source: Ty<'tcx>, + target: Ty<'tcx>, + normalize: impl Fn(Ty<'tcx>) -> Ty<'tcx>) + -> (Ty<'tcx>, Ty<'tcx>) + { let (mut a, mut b) = (source, target); loop { match (&a.sty, &b.sty) { @@ -320,6 +385,22 @@ impl<'tcx> TyCtxt<'tcx> { break; } }, + (ty::Projection(_), _) | (ty::Opaque(..), _) | + (_, ty::Projection(_)) | (_, ty::Opaque(..)) => { + // If either side is a projection, attempt to + // progress via normalization. (Should be safe to + // apply to both sides as normalization is + // idempotent.) + let a_norm = normalize(a); + let b_norm = normalize(b); + if a == a_norm && b == b_norm { + break; + } else { + a = a_norm; + b = b_norm; + } + } + _ => break, } } diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index ca7e17ec97a39..00471095f2f35 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -128,7 +128,8 @@ pub fn unsized_info<'tcx, Cx: CodegenMethods<'tcx>>( target: Ty<'tcx>, old_info: Option, ) -> Cx::Value { - let (source, target) = cx.tcx().struct_lockstep_tails(source, target); + let (source, target) = + cx.tcx().struct_lockstep_tails_erasing_lifetimes(source, target, cx.param_env()); match (&source.sty, &target.sty) { (&ty::Array(_, len), &ty::Slice(_)) => { cx.const_usize(len.unwrap_usize(cx.tcx())) diff --git a/src/librustc_codegen_ssa/traits/type_.rs b/src/librustc_codegen_ssa/traits/type_.rs index aa38d8d51848d..13f72e23819a1 100644 --- a/src/librustc_codegen_ssa/traits/type_.rs +++ b/src/librustc_codegen_ssa/traits/type_.rs @@ -77,11 +77,12 @@ pub trait DerivedTypeMethods<'tcx>: BaseTypeMethods<'tcx> + MiscMethods<'tcx> { } fn type_has_metadata(&self, ty: Ty<'tcx>) -> bool { - if ty.is_sized(self.tcx().at(DUMMY_SP), ty::ParamEnv::reveal_all()) { + let param_env = ty::ParamEnv::reveal_all(); + if ty.is_sized(self.tcx().at(DUMMY_SP), param_env) { return false; } - let tail = self.tcx().struct_tail(ty); + let tail = self.tcx().struct_tail_erasing_lifetimes(ty, param_env); match tail.sty { ty::Foreign(..) => false, ty::Str | ty::Slice(..) | ty::Dynamic(..) => true, diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 3ef525979f8c9..980697360eb75 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -270,7 +270,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { dty: Ty<'tcx>, ) -> InterpResult<'tcx> { // A -> A conversion - let (src_pointee_ty, dest_pointee_ty) = self.tcx.struct_lockstep_tails(sty, dty); + let (src_pointee_ty, dest_pointee_ty) = + self.tcx.struct_lockstep_tails_erasing_lifetimes(sty, dty, self.param_env); match (&src_pointee_ty.sty, &dest_pointee_ty.sty) { (&ty::Array(_, length), &ty::Slice(_)) => { diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index f0d64e217a28f..bcd36ac547c73 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -146,7 +146,9 @@ for let value = self.ecx.read_immediate(mplace.into())?; // Handle trait object vtables if let Ok(meta) = value.to_meta() { - if let ty::Dynamic(..) = self.ecx.tcx.struct_tail(referenced_ty).sty { + if let ty::Dynamic(..) = + self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.param_env).sty + { if let Ok(vtable) = meta.unwrap().to_ptr() { // explitly choose `Immutable` here, since vtables are immutable, even // if the reference of the fat pointer is mutable diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 34892f5b8ca01..423fd9036bf71 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -361,7 +361,8 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> "uninitialized data in fat pointer metadata", self.path); let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; if layout.is_unsized() { - let tail = self.ecx.tcx.struct_tail(layout.ty); + let tail = self.ecx.tcx.struct_tail_erasing_lifetimes(layout.ty, + self.ecx.param_env); match tail.sty { ty::Dynamic(..) => { let vtable = meta.unwrap(); diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index bcc6ad4eac29f..6e9390f77508b 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -851,12 +851,13 @@ fn find_vtable_types_for_unsizing<'tcx>( target_ty: Ty<'tcx>, ) -> (Ty<'tcx>, Ty<'tcx>) { let ptr_vtable = |inner_source: Ty<'tcx>, inner_target: Ty<'tcx>| { + let param_env = ty::ParamEnv::reveal_all(); let type_has_metadata = |ty: Ty<'tcx>| -> bool { use syntax_pos::DUMMY_SP; - if ty.is_sized(tcx.at(DUMMY_SP), ty::ParamEnv::reveal_all()) { + if ty.is_sized(tcx.at(DUMMY_SP), param_env) { return false; } - let tail = tcx.struct_tail(ty); + let tail = tcx.struct_tail_erasing_lifetimes(ty, param_env); match tail.sty { ty::Foreign(..) => false, ty::Str | ty::Slice(..) | ty::Dynamic(..) => true, @@ -866,7 +867,7 @@ fn find_vtable_types_for_unsizing<'tcx>( if type_has_metadata(inner_source) { (inner_source, inner_target) } else { - tcx.struct_lockstep_tails(inner_source, inner_target) + tcx.struct_lockstep_tails_erasing_lifetimes(inner_source, inner_target, param_env) } }; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 4acba6d38fa25..c35136ad6df94 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -315,7 +315,7 @@ impl<'a, 'tcx> Expectation<'tcx> { /// See the test case `test/run-pass/coerce-expect-unsized.rs` and #20169 /// for examples of where this comes up,. fn rvalue_hint(fcx: &FnCtxt<'a, 'tcx>, ty: Ty<'tcx>) -> Expectation<'tcx> { - match fcx.tcx.struct_tail(ty).sty { + match fcx.tcx.struct_tail_without_normalization(ty).sty { ty::Slice(_) | ty::Str | ty::Dynamic(..) => { ExpectRvalueLikeUnsized(ty) } diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index a41f4ec91a426..68e5e7d4fd245 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -366,7 +366,8 @@ fn check_item_type( let mut forbid_unsized = true; if allow_foreign_ty { - if let ty::Foreign(_) = fcx.tcx.struct_tail(item_ty).sty { + let tail = fcx.tcx.struct_tail_erasing_lifetimes(item_ty, fcx.param_env); + if let ty::Foreign(_) = tail.sty { forbid_unsized = false; } } From e4b8af5d607b465f11615abefb5bbae20486887e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 11 Jul 2019 13:54:54 +0200 Subject: [PATCH 4/8] Regression test for issue 60431. --- ...ue-60431-unsized-tail-behind-projection.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs diff --git a/src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs b/src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs new file mode 100644 index 0000000000000..65845d2c9fece --- /dev/null +++ b/src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs @@ -0,0 +1,35 @@ +// rust-lang/rust#60431: This is a scenario where to determine the size of +// `&Ref`, we need to know the concrete type of the last field in +// `Ref` (i.e. its "struct tail"), and determining that concrete type +// requires normalizing `Obstack::Dyn`. +// +// The old "struct tail" computation did not perform such normalization, and so +// the compiler would ICE when trying to figure out if `Ref` is a +// dynamically-sized type (DST). + +// run-pass + +use std::mem; + +pub trait Arena { + type Dyn : ?Sized; +} + +pub struct DynRef { + _dummy: [()], +} + +pub struct Ref { + _value: u8, + _dyn_arena: A::Dyn, +} + +pub struct Obstack; + +impl Arena for Obstack { + type Dyn = DynRef; +} + +fn main() { + assert_eq!(mem::size_of::<&Ref>(), mem::size_of::<&[()]>()); +} From 3c8279a2246c8bbb63f7d6aeac144b3a8adbe755 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 12 Jul 2019 11:34:23 +0200 Subject: [PATCH 5/8] Update docs to reflect review feedback. --- src/librustc/ty/util.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 871aeb78af517..635845a65c2af 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -270,6 +270,10 @@ impl<'tcx> TyCtxt<'tcx> { /// Returns the deeply last field of nested structures, or the same type if /// not a structure at all. Corresponds to the only possible unsized field, /// and its type can be used to determine unsizing strategy. + /// + /// Should only be called if `ty` has no inference variables and does not + /// need its lifetimes preserved (e.g. as part of codegen); otherwise + /// normalization attempt may cause compiler bugs. pub fn struct_tail_erasing_lifetimes(self, ty: Ty<'tcx>, param_env: ty::ParamEnv<'tcx>) @@ -287,8 +291,8 @@ impl<'tcx> TyCtxt<'tcx> { /// handle `::Assoc` and `impl Trait`); pass the identity /// function to indicate no normalization should take place. /// - /// See also `struct_tail_erasing_lifetimes`, which is what callers running - /// after type checking should use. + /// See also `struct_tail_erasing_lifetimes`, which is suitable for use + /// during codegen. pub fn struct_tail_with_normalize(self, mut ty: Ty<'tcx>, normalize: impl Fn(Ty<'tcx>) -> Ty<'tcx>) @@ -337,7 +341,8 @@ impl<'tcx> TyCtxt<'tcx> { /// For `(Foo>, Foo)`, the result will be `(Foo, Trait)`, /// whereas struct_tail produces `T`, and `Trait`, respectively. /// - /// Must only be called after type-checking is complete; otherwise + /// Should only be called if the types have no inference variables and do + /// not need their lifetimes preserved (e.g. as part of codegen); otherwise /// normalization attempt may cause compiler bugs. pub fn struct_lockstep_tails_erasing_lifetimes(self, source: Ty<'tcx>, @@ -356,8 +361,8 @@ impl<'tcx> TyCtxt<'tcx> { /// For `(Foo>, Foo)`, the result will be `(Foo, Trait)`, /// whereas struct_tail produces `T`, and `Trait`, respectively. /// - /// See also struct_lockstep_tails_erasing_lifetimes, which - /// is what callers running after type checking should use. + /// See also `struct_lockstep_tails_erasing_lifetimes`, which is suitable for use + /// during codegen. pub fn struct_lockstep_tails_with_normalize(self, source: Ty<'tcx>, target: Ty<'tcx>, From 278e5fd2152bfba3234f97560a378bdb03e24a3d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 6 Jun 2019 07:56:07 -0700 Subject: [PATCH 6/8] rustbuild: Improve assert about building tools once In developing #61557 I noticed that there were two parts of our tools that were rebuilt twice on CI. One was rustfmt fixed in #61557, but another was Cargo. The actual fix for Cargo's double compile was rust-lang/cargo#7010 and took some time to propagate here. In an effort to continue to assert that Cargo is itself not compiled twice, I updated the assertion in rustbuild at the time of working on #61557 but couldn't land it because the fix wouldn't be ready until the next bootstrap. The next bootstrap is now here, so the fix can now land! This does not change the behavior of rustbuild but it is intended to catch the previous iteration of compiling cargo twice. The main update here was to consider more files than those in `$target/release/deps` but also consider those in `$target/release`. That's where, for example, `libcargo.rlib` shows up and it's the file we learn about, and that's what we want to deduplicate. --- src/bootstrap/tool.rs | 69 ++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index bd77f7a91d94a..a9269a7ad24ab 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -109,36 +109,63 @@ impl Step for ToolBuild { continue } - // Don't worry about libs that turn out to be host dependencies - // or build scripts, we only care about target dependencies that - // are in `deps`. - if let Some(maybe_target) = val.1 - .parent() // chop off file name - .and_then(|p| p.parent()) // chop off `deps` - .and_then(|p| p.parent()) // chop off `release` - .and_then(|p| p.file_name()) - .and_then(|p| p.to_str()) - { - if maybe_target != &*target { - continue + // Don't worry about compiles that turn out to be host + // dependencies or build scripts. To skip these we look for + // anything that goes in `.../release/deps` but *doesn't* go in + // `$target/release/deps`. This ensure that outputs in + // `$target/release` are still considered candidates for + // deduplication. + if let Some(parent) = val.1.parent() { + if parent.ends_with("release/deps") { + let maybe_target = parent + .parent() + .and_then(|p| p.parent()) + .and_then(|p| p.file_name()) + .and_then(|p| p.to_str()) + .unwrap(); + if maybe_target != &*target { + continue; + } } } + // Record that we've built an artifact for `id`, and if one was + // already listed then we need to see if we reused the same + // artifact or produced a duplicate. let mut artifacts = builder.tool_artifacts.borrow_mut(); let prev_artifacts = artifacts .entry(target) .or_default(); - if let Some(prev) = prev_artifacts.get(&*id) { - if prev.1 != val.1 { - duplicates.push(( - id.to_string(), - val, - prev.clone(), - )); + let prev = match prev_artifacts.get(&*id) { + Some(prev) => prev, + None => { + prev_artifacts.insert(id.to_string(), val); + continue; } - return + }; + if prev.1 == val.1 { + return; // same path, same artifact } - prev_artifacts.insert(id.to_string(), val); + + // If the paths are different and one of them *isn't* inside of + // `release/deps`, then it means it's probably in + // `$target/release`, or it's some final artifact like + // `libcargo.rlib`. In these situations Cargo probably just + // copied it up from `$target/release/deps/libcargo-xxxx.rlib`, + // so if the features are equal we can just skip it. + let prev_no_hash = prev.1.parent().unwrap().ends_with("release/deps"); + let val_no_hash = val.1.parent().unwrap().ends_with("release/deps"); + if prev.2 == val.2 || !prev_no_hash || !val_no_hash { + return; + } + + // ... and otherwise this looks like we duplicated some sort of + // compilation, so record it to generate an error later. + duplicates.push(( + id.to_string(), + val, + prev.clone(), + )); } }); From 313ba7c4d11f52b808f29678b008bb0962f9af91 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 13 Jul 2019 09:55:14 +0100 Subject: [PATCH 7/8] Make `newtype_index` hygienic and use allow_internal_unstable --- src/librustc/lib.rs | 2 -- src/librustc_data_structures/indexed_vec.rs | 33 +++++++++++---------- src/librustc_mir/lib.rs | 2 -- src/librustc_target/abi/mod.rs | 1 + src/librustc_target/lib.rs | 5 ---- src/libsyntax/lib.rs | 2 -- src/test/run-pass-fulldeps/newtype_index.rs | 6 ++-- 7 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index dc26140ace5a5..63e0107a4d882 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -49,7 +49,6 @@ #![feature(optin_builtin_traits)] #![feature(range_is_empty)] #![feature(rustc_diagnostic_macros)] -#![feature(rustc_attrs)] #![feature(slice_patterns)] #![feature(specialization)] #![feature(unboxed_closures)] @@ -57,7 +56,6 @@ #![feature(trace_macros)] #![feature(trusted_len)] #![feature(vec_remove_item)] -#![feature(step_trait)] #![feature(stmt_expr_attributes)] #![feature(integer_atomics)] #![feature(test)] diff --git a/src/librustc_data_structures/indexed_vec.rs b/src/librustc_data_structures/indexed_vec.rs index b3a810a622d03..c3c76e8160615 100644 --- a/src/librustc_data_structures/indexed_vec.rs +++ b/src/librustc_data_structures/indexed_vec.rs @@ -57,12 +57,13 @@ impl Idx for u32 { /// `u32::MAX`. You can also customize things like the `Debug` impl, /// what traits are derived, and so forth via the macro. #[macro_export] +#[allow_internal_unstable(step_trait, rustc_attrs)] macro_rules! newtype_index { // ---- public rules ---- // Use default constants ($(#[$attrs:meta])* $v:vis struct $name:ident { .. }) => ( - newtype_index!( + $crate::newtype_index!( // Leave out derives marker so we can use its absence to ensure it comes first @attrs [$(#[$attrs])*] @type [$name] @@ -74,7 +75,7 @@ macro_rules! newtype_index { // Define any constants ($(#[$attrs:meta])* $v:vis struct $name:ident { $($tokens:tt)+ }) => ( - newtype_index!( + $crate::newtype_index!( // Leave out derives marker so we can use its absence to ensure it comes first @attrs [$(#[$attrs])*] @type [$name] @@ -258,7 +259,7 @@ macro_rules! newtype_index { } } - newtype_index!( + $crate::newtype_index!( @handle_debug @derives [$($derives,)*] @type [$type] @@ -294,7 +295,7 @@ macro_rules! newtype_index { @derives [$_derive:ident, $($derives:ident,)*] @type [$type:ident] @debug_format [$debug_format:tt]) => ( - newtype_index!( + $crate::newtype_index!( @handle_debug @derives [$($derives,)*] @type [$type] @@ -309,7 +310,7 @@ macro_rules! newtype_index { @debug_format [$debug_format:tt] derive [$($derives:ident),*] $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @attrs [$(#[$attrs])*] @type [$type] @max [$max] @@ -329,7 +330,7 @@ macro_rules! newtype_index { derive [$($derives:ident,)+] ENCODABLE = custom $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @attrs [$(#[$attrs])*] @derives [$($derives,)+] @type [$type] @@ -348,7 +349,7 @@ macro_rules! newtype_index { @debug_format [$debug_format:tt] derive [$($derives:ident,)+] $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @derives [$($derives,)+ RustcEncodable,] @attrs [$(#[$attrs])*] @type [$type] @@ -356,7 +357,7 @@ macro_rules! newtype_index { @vis [$v] @debug_format [$debug_format] $($tokens)*); - newtype_index!(@decodable $type); + $crate::newtype_index!(@decodable $type); ); // The case where no derives are added, but encodable is overridden. Don't @@ -368,7 +369,7 @@ macro_rules! newtype_index { @debug_format [$debug_format:tt] ENCODABLE = custom $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @derives [] @attrs [$(#[$attrs])*] @type [$type] @@ -385,7 +386,7 @@ macro_rules! newtype_index { @vis [$v:vis] @debug_format [$debug_format:tt] $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @derives [RustcEncodable,] @attrs [$(#[$attrs])*] @type [$type] @@ -393,7 +394,7 @@ macro_rules! newtype_index { @vis [$v] @debug_format [$debug_format] $($tokens)*); - newtype_index!(@decodable $type); + $crate::newtype_index!(@decodable $type); ); (@decodable $type:ident) => ( @@ -420,7 +421,7 @@ macro_rules! newtype_index { @vis [$v:vis] @debug_format [$debug_format:tt] $name:ident = $constant:expr) => ( - newtype_index!( + $crate::newtype_index!( @derives [$($derives,)*] @attrs [$(#[$attrs])*] @type [$type] @@ -439,7 +440,7 @@ macro_rules! newtype_index { @debug_format [$debug_format:tt] $(#[doc = $doc:expr])* const $name:ident = $constant:expr) => ( - newtype_index!( + $crate::newtype_index!( @derives [$($derives,)*] @attrs [$(#[$attrs])*] @type [$type] @@ -458,7 +459,7 @@ macro_rules! newtype_index { @debug_format [$debug_format:tt] MAX = $max:expr, $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @derives [$($derives,)*] @attrs [$(#[$attrs])*] @type [$type] @@ -477,7 +478,7 @@ macro_rules! newtype_index { @debug_format [$_debug_format:tt] DEBUG_FORMAT = $debug_format:tt, $($tokens:tt)*) => ( - newtype_index!( + $crate::newtype_index!( @derives [$($derives,)*] @attrs [$(#[$attrs])*] @type [$type] @@ -499,7 +500,7 @@ macro_rules! newtype_index { $($tokens:tt)*) => ( $(#[doc = $doc])* pub const $name: $type = $type::from_u32_const($constant); - newtype_index!( + $crate::newtype_index!( @derives [$($derives,)*] @attrs [$(#[$attrs])*] @type [$type] diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index 4a80534503a5d..f5e4661afa6b1 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -15,12 +15,10 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![feature(decl_macro)] #![feature(exhaustive_patterns)] #![feature(rustc_diagnostic_macros)] -#![feature(rustc_attrs)] #![feature(never_type)] #![feature(specialization)] #![feature(try_trait)] #![feature(unicode_internals)] -#![feature(step_trait)] #![feature(slice_concat_ext)] #![feature(trusted_len)] #![feature(try_blocks)] diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 55cb179144309..01586e92aeb1c 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -6,6 +6,7 @@ use crate::spec::Target; use std::fmt; use std::ops::{Add, Deref, Sub, Mul, AddAssign, Range, RangeInclusive}; +use rustc_data_structures::newtype_index; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use syntax_pos::symbol::{sym, Symbol}; diff --git a/src/librustc_target/lib.rs b/src/librustc_target/lib.rs index c1ec4e59ef239..dcd1eb5acdc85 100644 --- a/src/librustc_target/lib.rs +++ b/src/librustc_target/lib.rs @@ -11,9 +11,7 @@ #![feature(box_syntax)] #![feature(nll)] -#![feature(rustc_attrs)] #![feature(slice_patterns)] -#![feature(step_trait)] #![deny(rust_2018_idioms)] #![deny(unused_lifetimes)] @@ -23,8 +21,5 @@ #[allow(unused_extern_crates)] extern crate serialize as rustc_serialize; // used by deriving -#[macro_use] -extern crate rustc_data_structures; - pub mod abi; pub mod spec; diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index ee7fb97ffd718..591f2fb599be3 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -18,9 +18,7 @@ #![feature(label_break_value)] #![feature(mem_take)] #![feature(nll)] -#![feature(rustc_attrs)] #![feature(rustc_diagnostic_macros)] -#![feature(step_trait)] #![feature(try_trait)] #![feature(unicode_internals)] diff --git a/src/test/run-pass-fulldeps/newtype_index.rs b/src/test/run-pass-fulldeps/newtype_index.rs index 18b845eeecb44..1192a44a6eecf 100644 --- a/src/test/run-pass-fulldeps/newtype_index.rs +++ b/src/test/run-pass-fulldeps/newtype_index.rs @@ -1,9 +1,9 @@ -#![feature(rustc_attrs, rustc_private, step_trait)] +#![feature(rustc_private)] -#[macro_use] extern crate rustc_data_structures; +extern crate rustc_data_structures; extern crate serialize as rustc_serialize; -use rustc_data_structures::indexed_vec::Idx; +use rustc_data_structures::{newtype_index, indexed_vec::Idx}; newtype_index!(struct MyIdx { MAX = 0xFFFF_FFFA }); From 199931ce910776e6cd035da8cdf1dd81f4d411ba Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 13 Jul 2019 10:02:40 +0100 Subject: [PATCH 8/8] Make `register_[long_]diagnostics` hygienic --- src/librustc_lint/error_codes.rs | 2 +- src/librustc_metadata/error_codes.rs | 2 +- src/librustc_passes/error_codes.rs | 2 +- src/librustc_plugin/error_codes.rs | 2 +- src/librustc_resolve/error_codes.rs | 2 +- src/libsyntax/diagnostics/macros.rs | 8 ++++---- src/libsyntax_ext/error_codes.rs | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/librustc_lint/error_codes.rs b/src/librustc_lint/error_codes.rs index 3165673111cca..d7c39b780bfdf 100644 --- a/src/librustc_lint/error_codes.rs +++ b/src/librustc_lint/error_codes.rs @@ -1,4 +1,4 @@ -use syntax::{register_diagnostic, register_diagnostics}; +use syntax::register_diagnostics; register_diagnostics! { E0721, // `await` keyword diff --git a/src/librustc_metadata/error_codes.rs b/src/librustc_metadata/error_codes.rs index 9ac582ebc42da..7c63173659135 100644 --- a/src/librustc_metadata/error_codes.rs +++ b/src/librustc_metadata/error_codes.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] -use syntax::{register_diagnostic, register_diagnostics, register_long_diagnostics}; +use syntax::{register_diagnostics, register_long_diagnostics}; register_long_diagnostics! { E0454: r##" diff --git a/src/librustc_passes/error_codes.rs b/src/librustc_passes/error_codes.rs index e3c6b16703a4a..36ebe5cf455ff 100644 --- a/src/librustc_passes/error_codes.rs +++ b/src/librustc_passes/error_codes.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] -use syntax::{register_diagnostic, register_diagnostics, register_long_diagnostics}; +use syntax::{register_diagnostics, register_long_diagnostics}; register_long_diagnostics! { /* diff --git a/src/librustc_plugin/error_codes.rs b/src/librustc_plugin/error_codes.rs index 68462bd83ef60..9e76f52a111b5 100644 --- a/src/librustc_plugin/error_codes.rs +++ b/src/librustc_plugin/error_codes.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] -use syntax::{register_diagnostic, register_diagnostics, register_long_diagnostics}; +use syntax::{register_diagnostics, register_long_diagnostics}; register_long_diagnostics! { diff --git a/src/librustc_resolve/error_codes.rs b/src/librustc_resolve/error_codes.rs index 7cd26dce1447c..15194a5d1463b 100644 --- a/src/librustc_resolve/error_codes.rs +++ b/src/librustc_resolve/error_codes.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] -use syntax::{register_diagnostic, register_diagnostics, register_long_diagnostics}; +use syntax::{register_diagnostics, register_long_diagnostics}; // Error messages for EXXXX errors. Each message should start and end with a // new line, and be wrapped to 80 characters. In vim you can `:set tw=80` and diff --git a/src/libsyntax/diagnostics/macros.rs b/src/libsyntax/diagnostics/macros.rs index 6f7493ad59726..b754d0833761e 100644 --- a/src/libsyntax/diagnostics/macros.rs +++ b/src/libsyntax/diagnostics/macros.rs @@ -170,19 +170,19 @@ macro_rules! help { #[macro_export] macro_rules! register_diagnostics { ($($code:tt),*) => ( - $(register_diagnostic! { $code })* + $($crate::register_diagnostic! { $code })* ); ($($code:tt),*,) => ( - $(register_diagnostic! { $code })* + $($crate::register_diagnostic! { $code })* ) } #[macro_export] macro_rules! register_long_diagnostics { ($($code:tt: $description:tt),*) => ( - $(register_diagnostic! { $code, $description })* + $($crate::register_diagnostic! { $code, $description })* ); ($($code:tt: $description:tt),*,) => ( - $(register_diagnostic! { $code, $description })* + $($crate::register_diagnostic! { $code, $description })* ) } diff --git a/src/libsyntax_ext/error_codes.rs b/src/libsyntax_ext/error_codes.rs index 9bbd9fdec17d6..9ec551b439529 100644 --- a/src/libsyntax_ext/error_codes.rs +++ b/src/libsyntax_ext/error_codes.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] -use syntax::{register_diagnostic, register_long_diagnostics}; +use syntax::register_long_diagnostics; // Error messages for EXXXX errors. // Each message should start and end with a new line, and be wrapped to 80 characters.