From 93cbad6ed5f6a40bdd1e8cee6a9b1a39f17ab166 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 17:44:33 +0200 Subject: [PATCH 01/28] Add documentation to point to `!is_dir` instead of `is_file` --- src/libstd/fs.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index f4c164a324e32..c2bea8d9d638f 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1307,6 +1307,12 @@ impl FileType { /// [`is_dir`] and [`is_symlink`]; only zero or one of these /// tests may pass. /// + /// This property means it is often more useful to use `!file_type.is_dir()` + /// than `file_type.is_file()` when your goal is to read bytes from a + /// source: the former includes symlink and pipes when the latter does not, + /// meaning you will break workflows like `diff <( prog_a ) <( prog_b )` on + /// a Unix-like system for example. + /// /// [`is_dir`]: struct.FileType.html#method.is_dir /// [`is_symlink`]: struct.FileType.html#method.is_symlink /// From ec63f9d99b4faec04db0f924c24be9529f4febed Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 18:15:57 +0200 Subject: [PATCH 02/28] Added the note to Metadata too --- src/libstd/fs.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index c2bea8d9d638f..a9c481dfb809f 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1033,6 +1033,12 @@ impl Metadata { /// [`is_dir`], and will be false for symlink metadata /// obtained from [`symlink_metadata`]. /// + /// This property means it is often more useful to use `!file_type.is_dir()` + /// than `file_type.is_file()` when your goal is to read bytes from a + /// source: the former includes symlink and pipes when the latter does not, + /// meaning you will break workflows like `diff <( prog_a ) <( prog_b )` on + /// a Unix-like system for example. + /// /// [`is_dir`]: struct.Metadata.html#method.is_dir /// [`symlink_metadata`]: fn.symlink_metadata.html /// From c1243dbcd96f43d013e38f01efe91eb35b81fa18 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 18:17:00 +0200 Subject: [PATCH 03/28] Make a note about is_dir vs is_file in Path too --- src/libstd/path.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 8ff7508ba6457..35f6b5838a92a 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2503,11 +2503,15 @@ impl Path { /// # See Also /// /// This is a convenience function that coerces errors to false. If you want to - /// check errors, call [fs::metadata] and handle its Result. Then call - /// [fs::Metadata::is_file] if it was Ok. + /// check errors, call [`fs::metadata`] and handle its Result. Then call + /// [`fs::Metadata::is_file`] if it was Ok. /// - /// [fs::metadata]: ../../std/fs/fn.metadata.html - /// [fs::Metadata::is_file]: ../../std/fs/struct.Metadata.html#method.is_file + /// Note that the explanation about using `!is_dir` instead of `is_file` + /// that is present in the [`fs::Metadata`] documentation also applies here. + /// + /// [`fs::metadata`]: ../../std/fs/fn.metadata.html + /// [`fs::Metadata`]: ../../std/fs/struct.Metadata.html + /// [`fs::Metadata::is_file`]: ../../std/fs/struct.Metadata.html#method.is_file #[stable(feature = "path_ext", since = "1.5.0")] pub fn is_file(&self) -> bool { fs::metadata(self).map(|m| m.is_file()).unwrap_or(false) From 810f309ff30fe7a75917f9e5359074dc991b4590 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 30 May 2020 23:46:21 +0200 Subject: [PATCH 04/28] MIR sanity check: validate types on assignment --- src/librustc_mir/transform/validate.rs | 54 ++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 625f40cd79206..3d48a2387a88e 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -7,7 +7,7 @@ use rustc_middle::{ BasicBlock, Body, Location, Operand, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }, - ty::{self, ParamEnv, TyCtxt}, + ty::{self, fold::BottomUpFolder, ParamEnv, Ty, TyCtxt, TypeFoldable}, }; #[derive(Copy, Clone, Debug)] @@ -83,6 +83,40 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } } +/// Check if src can be assigned into dest. +/// This is not precise, it will accept some incorrect assignments. +fn mir_assign_valid_types<'tcx>(tcx: TyCtxt<'tcx>, src: Ty<'tcx>, dest: Ty<'tcx>) -> bool { + if src == dest { + // Equal types, all is good. + return true; + } + + // Type-changing assignments can happen for (at least) two reasons: + // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment. + // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types + // with their late-bound lifetimes are still around and can lead to type differences. + // Normalize both of them away. + // FIXME: Share this code with `interpret/eval_context.rs`. + let normalize = |ty: Ty<'tcx>| { + ty.fold_with(&mut BottomUpFolder { + tcx, + // Normalize all references to immutable. + ty_op: |ty| match ty.kind { + ty::Ref(_, pointee, _) => tcx.mk_imm_ref(tcx.lifetimes.re_erased, pointee), + _ => ty, + }, + // We just erase all late-bound lifetimes, but this is not fully correct (FIXME): + // lifetimes in invariant positions could matter (e.g. through associated types). + // But that just means we miss some potential incompatible types, it will not + // lead to wrong errors. + lt_op: |_| tcx.lifetimes.re_erased, + // Leave consts unchanged. + ct_op: |ct| ct, + }) + }; + normalize(src) == normalize(dest) +} + impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) { // `Operand::Copy` is only supposed to be used with `Copy` types. @@ -99,9 +133,23 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { - // The sides of an assignment must not alias. Currently this just checks whether the places - // are identical. if let StatementKind::Assign(box (dest, rvalue)) = &statement.kind { + // LHS and RHS of the assignment must have the same type. + let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty; + let right_ty = rvalue.ty(&self.body.local_decls, self.tcx); + if !mir_assign_valid_types(self.tcx, right_ty, left_ty) { + self.fail( + location, + format!( + "encountered `Assign` statement with incompatible types:\n\ + left-hand side has type: {}\n\ + right-hand side has type: {}", + left_ty, right_ty, + ), + ); + } + // The sides of an assignment must not alias. Currently this just checks whether the places + // are identical. match rvalue { Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) => { if dest == src { From 50d7deac4de3bfde44a634ff4dabf3115f694c79 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 May 2020 09:54:25 +0200 Subject: [PATCH 05/28] prepare visit_statement for checking more kinds of statements --- src/librustc_mir/transform/validate.rs | 53 ++++++++++++++------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 3d48a2387a88e..051ce9e6b1ef8 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -133,34 +133,37 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { - if let StatementKind::Assign(box (dest, rvalue)) = &statement.kind { - // LHS and RHS of the assignment must have the same type. - let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty; - let right_ty = rvalue.ty(&self.body.local_decls, self.tcx); - if !mir_assign_valid_types(self.tcx, right_ty, left_ty) { - self.fail( - location, - format!( - "encountered `Assign` statement with incompatible types:\n\ - left-hand side has type: {}\n\ - right-hand side has type: {}", - left_ty, right_ty, - ), - ); - } - // The sides of an assignment must not alias. Currently this just checks whether the places - // are identical. - match rvalue { - Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) => { - if dest == src { - self.fail( - location, - "encountered `Assign` statement with overlapping memory", - ); + match &statement.kind { + StatementKind::Assign(box (dest, rvalue)) => { + // LHS and RHS of the assignment must have the same type. + let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty; + let right_ty = rvalue.ty(&self.body.local_decls, self.tcx); + if !mir_assign_valid_types(self.tcx, right_ty, left_ty) { + self.fail( + location, + format!( + "encountered `Assign` statement with incompatible types:\n\ + left-hand side has type: {}\n\ + right-hand side has type: {}", + left_ty, right_ty, + ), + ); + } + // The sides of an assignment must not alias. Currently this just checks whether the places + // are identical. + match rvalue { + Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) => { + if dest == src { + self.fail( + location, + "encountered `Assign` statement with overlapping memory", + ); + } } + _ => {} } - _ => {} } + _ => {} } } From 93e3552d040edfa67cdedfe2fe4a44fe0c4ead59 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 May 2020 15:02:33 +0200 Subject: [PATCH 06/28] also normalize constants when comparing types --- src/librustc_mir/interpret/eval_context.rs | 1 + src/librustc_mir/transform/validate.rs | 68 +++++++++++----------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 22f4691c22b3d..1a6ed41ba47e5 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -239,6 +239,7 @@ pub(super) fn mir_assign_valid_types<'tcx>( // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types // with their late-bound lifetimes are still around and can lead to type differences. // Normalize both of them away. + // Also see the related but slightly different pre-monomorphization method in `transform/validate.rs`. let normalize = |ty: Ty<'tcx>| { ty.fold_with(&mut BottomUpFolder { tcx, diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 051ce9e6b1ef8..14c67c2372c9f 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -81,40 +81,42 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { self.fail(location, format!("encountered jump to invalid basic block {:?}", bb)) } } -} -/// Check if src can be assigned into dest. -/// This is not precise, it will accept some incorrect assignments. -fn mir_assign_valid_types<'tcx>(tcx: TyCtxt<'tcx>, src: Ty<'tcx>, dest: Ty<'tcx>) -> bool { - if src == dest { - // Equal types, all is good. - return true; - } + /// Check if src can be assigned into dest. + /// This is not precise, it will accept some incorrect assignments. + fn mir_assign_valid_types(&self, src: Ty<'tcx>, dest: Ty<'tcx>) -> bool { + if src == dest { + // Equal types, all is good. + return true; + } - // Type-changing assignments can happen for (at least) two reasons: - // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment. - // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types - // with their late-bound lifetimes are still around and can lead to type differences. - // Normalize both of them away. - // FIXME: Share this code with `interpret/eval_context.rs`. - let normalize = |ty: Ty<'tcx>| { - ty.fold_with(&mut BottomUpFolder { - tcx, - // Normalize all references to immutable. - ty_op: |ty| match ty.kind { - ty::Ref(_, pointee, _) => tcx.mk_imm_ref(tcx.lifetimes.re_erased, pointee), - _ => ty, - }, - // We just erase all late-bound lifetimes, but this is not fully correct (FIXME): - // lifetimes in invariant positions could matter (e.g. through associated types). - // But that just means we miss some potential incompatible types, it will not - // lead to wrong errors. - lt_op: |_| tcx.lifetimes.re_erased, - // Leave consts unchanged. - ct_op: |ct| ct, - }) - }; - normalize(src) == normalize(dest) + // Type-changing assignments can happen for (at least) two reasons: + // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment. + // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types + // with their late-bound lifetimes are still around and can lead to type differences. + // Normalize both of them away. + // Also see the related but slightly different post-monomorphization method in `interpret/eval_context.rs`. + let normalize = |ty: Ty<'tcx>| { + ty.fold_with(&mut BottomUpFolder { + tcx: self.tcx, + // Normalize all references to immutable. + ty_op: |ty| match ty.kind { + ty::Ref(_, pointee, _) => { + self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, pointee) + } + _ => ty, + }, + // We just erase all late-bound lifetimes, but this is not fully correct (FIXME): + // lifetimes in invariant positions could matter (e.g. through associated types). + // But that just means we miss some potential incompatible types, it will not + // lead to wrong errors. + lt_op: |_| self.tcx.lifetimes.re_erased, + // Evaluate consts. + ct_op: |ct| ct.eval(self.tcx, self.param_env), + }) + }; + normalize(src) == normalize(dest) + } } impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { @@ -138,7 +140,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { // LHS and RHS of the assignment must have the same type. let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty; let right_ty = rvalue.ty(&self.body.local_decls, self.tcx); - if !mir_assign_valid_types(self.tcx, right_ty, left_ty) { + if !self.mir_assign_valid_types(right_ty, left_ty) { self.fail( location, format!( From 9576e307a7b8ac0c812fac927d247761662e7d1a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jun 2020 21:04:11 +0200 Subject: [PATCH 07/28] also normalize_erasing_regions --- src/librustc_mir/transform/validate.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 14c67c2372c9f..40189ca5c128c 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -89,6 +89,13 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // Equal types, all is good. return true; } + // Normalize projections and things like that. + let src = self.tcx.normalize_erasing_regions(self.param_env, src); + let dest = self.tcx.normalize_erasing_regions(self.param_env, dest); + // It's worth checking equality again. + if src == dest { + return true; + } // Type-changing assignments can happen for (at least) two reasons: // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment. From 8200771aa2cbd393a5beca819ac2462cf35e8d15 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Jun 2020 11:45:19 +0200 Subject: [PATCH 08/28] reveal_all when sanity-checking MIR assignment types --- src/librustc_mir/transform/validate.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 40189ca5c128c..b2fbb48eefea5 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -90,8 +90,11 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return true; } // Normalize projections and things like that. - let src = self.tcx.normalize_erasing_regions(self.param_env, src); - let dest = self.tcx.normalize_erasing_regions(self.param_env, dest); + // FIXME: We need to reveal_all, as some optimizations change types in ways + // that requires unfolding opaque types. + let param_env = self.param_env.with_reveal_all(); + let src = self.tcx.normalize_erasing_regions(param_env, src); + let dest = self.tcx.normalize_erasing_regions(param_env, dest); // It's worth checking equality again. if src == dest { return true; @@ -119,7 +122,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // lead to wrong errors. lt_op: |_| self.tcx.lifetimes.re_erased, // Evaluate consts. - ct_op: |ct| ct.eval(self.tcx, self.param_env), + ct_op: |ct| ct.eval(self.tcx, param_env), }) }; normalize(src) == normalize(dest) From 978470f711b3be3350a46d386a424e1dfb1ea148 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 21 Jun 2020 10:04:12 +0200 Subject: [PATCH 09/28] no need to normalize mutability any more --- src/librustc_mir/transform/validate.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index b2fbb48eefea5..81bdcc849e49c 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -100,22 +100,16 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return true; } - // Type-changing assignments can happen for (at least) two reasons: - // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment. - // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types - // with their late-bound lifetimes are still around and can lead to type differences. - // Normalize both of them away. - // Also see the related but slightly different post-monomorphization method in `interpret/eval_context.rs`. + // Type-changing assignments can happen when subtyping is used. While + // all normal lifetimes are erased, higher-ranked types with their + // late-bound lifetimes are still around and can lead to type + // differences. Normalize both of them away. + // Also see the related but slightly different post-monomorphization + // method in `interpret/eval_context.rs`. let normalize = |ty: Ty<'tcx>| { ty.fold_with(&mut BottomUpFolder { tcx: self.tcx, - // Normalize all references to immutable. - ty_op: |ty| match ty.kind { - ty::Ref(_, pointee, _) => { - self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, pointee) - } - _ => ty, - }, + ty_op: |ty| ty, // We just erase all late-bound lifetimes, but this is not fully correct (FIXME): // lifetimes in invariant positions could matter (e.g. through associated types). // But that just means we miss some potential incompatible types, it will not From 91f73fbca488973169b4f4b927323f712ad3d776 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 21 Jun 2020 13:49:53 +0200 Subject: [PATCH 10/28] use a TypeRelation to compare the types --- src/librustc_mir/transform/validate.rs | 98 +++++++++++++++++++++----- 1 file changed, 80 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 81bdcc849e49c..d0293131b263a 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -7,7 +7,11 @@ use rustc_middle::{ BasicBlock, Body, Location, Operand, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }, - ty::{self, fold::BottomUpFolder, ParamEnv, Ty, TyCtxt, TypeFoldable}, + ty::{ + self, + relate::{Relate, RelateResult, TypeRelation}, + ParamEnv, Ty, TyCtxt, + }, }; #[derive(Copy, Clone, Debug)] @@ -103,23 +107,81 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // Type-changing assignments can happen when subtyping is used. While // all normal lifetimes are erased, higher-ranked types with their // late-bound lifetimes are still around and can lead to type - // differences. Normalize both of them away. - // Also see the related but slightly different post-monomorphization - // method in `interpret/eval_context.rs`. - let normalize = |ty: Ty<'tcx>| { - ty.fold_with(&mut BottomUpFolder { - tcx: self.tcx, - ty_op: |ty| ty, - // We just erase all late-bound lifetimes, but this is not fully correct (FIXME): - // lifetimes in invariant positions could matter (e.g. through associated types). - // But that just means we miss some potential incompatible types, it will not - // lead to wrong errors. - lt_op: |_| self.tcx.lifetimes.re_erased, - // Evaluate consts. - ct_op: |ct| ct.eval(self.tcx, param_env), - }) - }; - normalize(src) == normalize(dest) + // differences. So we compare ignoring lifetimes. + struct LifetimeIgnoreRelation<'tcx> { + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + } + + impl TypeRelation<'tcx> for LifetimeIgnoreRelation<'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn param_env(&self) -> ty::ParamEnv<'tcx> { + self.param_env + } + + fn tag(&self) -> &'static str { + "librustc_mir::transform::validate" + } + + fn a_is_expected(&self) -> bool { + true + } + + fn relate_with_variance>( + &mut self, + _: ty::Variance, + a: &T, + b: &T, + ) -> RelateResult<'tcx, T> { + // Ignore variance, require types to be exactly the same. + self.relate(a, b) + } + + fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { + if a == b { + // Short-circuit. + return Ok(a); + } + ty::relate::super_relate_tys(self, a, b) + } + + fn regions( + &mut self, + a: ty::Region<'tcx>, + _b: ty::Region<'tcx>, + ) -> RelateResult<'tcx, ty::Region<'tcx>> { + // Ignore regions. + Ok(a) + } + + fn consts( + &mut self, + a: &'tcx ty::Const<'tcx>, + b: &'tcx ty::Const<'tcx>, + ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { + ty::relate::super_relate_consts(self, a, b) + } + + fn binders( + &mut self, + a: &ty::Binder, + b: &ty::Binder, + ) -> RelateResult<'tcx, ty::Binder> + where + T: Relate<'tcx>, + { + self.relate(a.skip_binder(), b.skip_binder())?; + Ok(a.clone()) + } + } + + // Instantiate and run relation. + let mut relator: LifetimeIgnoreRelation<'tcx> = + LifetimeIgnoreRelation { tcx: self.tcx, param_env }; + relator.relate(&src, &dest).is_ok() } } From 7754322bcc2852814592c47c3b76c53fefe95a4f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Jun 2020 09:00:40 +0200 Subject: [PATCH 11/28] fix typo Co-authored-by: Bastian Kauschke --- src/librustc_mir/transform/validate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index d0293131b263a..803954d258fa0 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -95,7 +95,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } // Normalize projections and things like that. // FIXME: We need to reveal_all, as some optimizations change types in ways - // that requires unfolding opaque types. + // that require unfolding opaque types. let param_env = self.param_env.with_reveal_all(); let src = self.tcx.normalize_erasing_regions(param_env, src); let dest = self.tcx.normalize_erasing_regions(param_env, dest); From 7f8fe6a9851aaf493c8657fe7e98145539d466dd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Jun 2020 09:17:33 +0200 Subject: [PATCH 12/28] also use relator in interpreter assignment sanity check --- src/librustc_mir/interpret/eval_context.rs | 41 ++---- src/librustc_mir/interpret/operand.rs | 4 +- src/librustc_mir/interpret/place.rs | 5 +- src/librustc_mir/transform/validate.rs | 162 +++++++++++---------- 4 files changed, 109 insertions(+), 103 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1a6ed41ba47e5..b673738cec580 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -15,7 +15,7 @@ use rustc_middle::mir::interpret::{ }; use rustc_middle::ty::layout::{self, TyAndLayout}; use rustc_middle::ty::{ - self, fold::BottomUpFolder, query::TyCtxtAt, subst::SubstsRef, Ty, TyCtxt, TypeFoldable, + self, query::TyCtxtAt, subst::SubstsRef, ParamEnv, Ty, TyCtxt, TypeFoldable, }; use rustc_span::{source_map::DUMMY_SP, Span}; use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout}; @@ -24,6 +24,7 @@ use super::{ Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy, ScalarMaybeUninit, StackPopJump, }; +use crate::transform::validate::equal_up_to_regions; use crate::util::storage::AlwaysLiveLocals; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -220,6 +221,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, /// This test should be symmetric, as it is primarily about layout compatibility. pub(super) fn mir_assign_valid_types<'tcx>( tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, src: TyAndLayout<'tcx>, dest: TyAndLayout<'tcx>, ) -> bool { @@ -234,29 +236,15 @@ pub(super) fn mir_assign_valid_types<'tcx>( return false; } - // Type-changing assignments can happen for (at least) two reasons: - // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment. - // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types - // with their late-bound lifetimes are still around and can lead to type differences. - // Normalize both of them away. - // Also see the related but slightly different pre-monomorphization method in `transform/validate.rs`. - let normalize = |ty: Ty<'tcx>| { - ty.fold_with(&mut BottomUpFolder { - tcx, - // Normalize all references to immutable. - ty_op: |ty| match ty.kind { - ty::Ref(_, pointee, _) => tcx.mk_imm_ref(tcx.lifetimes.re_erased, pointee), - _ => ty, - }, - // We just erase all late-bound lifetimes, but this is not fully correct (FIXME): - // lifetimes in invariant positions could matter (e.g. through associated types). - // We rely on the fact that layout was confirmed to be equal above. - lt_op: |_| tcx.lifetimes.re_erased, - // Leave consts unchanged. - ct_op: |ct| ct, - }) - }; - normalize(src.ty) == normalize(dest.ty) + // Type-changing assignments can happen when subtyping is used. While + // all normal lifetimes are erased, higher-ranked types with their + // late-bound lifetimes are still around and can lead to type + // differences. So we compare ignoring lifetimes. + // + // Note that this is not fully correct (FIXME): + // lifetimes in invariant positions could matter (e.g. through associated types). + // We rely on the fact that layout was confirmed to be equal above. + equal_up_to_regions(tcx, param_env, src.ty, dest.ty) } /// Use the already known layout if given (but sanity check in debug mode), @@ -264,6 +252,7 @@ pub(super) fn mir_assign_valid_types<'tcx>( #[cfg_attr(not(debug_assertions), inline(always))] pub(super) fn from_known_layout<'tcx>( tcx: TyCtxtAt<'tcx>, + param_env: ParamEnv<'tcx>, known_layout: Option>, compute: impl FnOnce() -> InterpResult<'tcx, TyAndLayout<'tcx>>, ) -> InterpResult<'tcx, TyAndLayout<'tcx>> { @@ -272,7 +261,7 @@ pub(super) fn from_known_layout<'tcx>( Some(known_layout) => { if cfg!(debug_assertions) { let check_layout = compute()?; - if !mir_assign_valid_types(tcx.tcx, check_layout, known_layout) { + if !mir_assign_valid_types(tcx.tcx, param_env, check_layout, known_layout) { span_bug!( tcx.span, "expected type differs from actual type.\nexpected: {:?}\nactual: {:?}", @@ -476,7 +465,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // have to support that case (mostly by skipping all caching). match frame.locals.get(local).and_then(|state| state.layout.get()) { None => { - let layout = from_known_layout(self.tcx, layout, || { + let layout = from_known_layout(self.tcx, self.param_env, layout, || { let local_ty = frame.body.local_decls[local].ty; let local_ty = self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty); diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 35e433c4bd5cd..27fa9b2c17c57 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -472,6 +472,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Sanity-check the type we ended up with. debug_assert!(mir_assign_valid_types( *self.tcx, + self.param_env, self.layout_of(self.subst_from_current_frame_and_normalize_erasing_regions( place.ty(&self.frame().body.local_decls, *self.tcx).ty ))?, @@ -554,7 +555,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // documentation). let val_val = M::adjust_global_const(self, val_val)?; // Other cases need layout. - let layout = from_known_layout(self.tcx, layout, || self.layout_of(val.ty))?; + let layout = + from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(val.ty))?; let op = match val_val { ConstValue::ByRef { alloc, offset } => { let id = self.tcx.create_memory_alloc(alloc); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 396aec0a8f89f..98a1cea97e220 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -652,6 +652,7 @@ where // Sanity-check the type we ended up with. debug_assert!(mir_assign_valid_types( *self.tcx, + self.param_env, self.layout_of(self.subst_from_current_frame_and_normalize_erasing_regions( place.ty(&self.frame().body.local_decls, *self.tcx).ty ))?, @@ -855,7 +856,7 @@ where ) -> InterpResult<'tcx> { // We do NOT compare the types for equality, because well-typed code can // actually "transmute" `&mut T` to `&T` in an assignment without a cast. - if !mir_assign_valid_types(*self.tcx, src.layout, dest.layout) { + if !mir_assign_valid_types(*self.tcx, self.param_env, src.layout, dest.layout) { span_bug!( self.cur_span(), "type mismatch when copying!\nsrc: {:?},\ndest: {:?}", @@ -912,7 +913,7 @@ where src: OpTy<'tcx, M::PointerTag>, dest: PlaceTy<'tcx, M::PointerTag>, ) -> InterpResult<'tcx> { - if mir_assign_valid_types(*self.tcx, src.layout, dest.layout) { + if mir_assign_valid_types(*self.tcx, self.param_env, src.layout, dest.layout) { // Fast path: Just use normal `copy_op` return self.copy_op(src, dest); } diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 803954d258fa0..5f0edd64c47e1 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -32,6 +32,93 @@ impl<'tcx> MirPass<'tcx> for Validator { } } +/// Returns whether the two types are equal up to lifetimes. +/// All lifetimes, including higher-ranked ones, get ignored for this comparison. +/// (This is unlike the `erasing_regions` methods, which keep higher-ranked lifetimes for soundness reasons.) +/// +/// The point of this function is to approximate "equal up to subtyping". However, +/// the approximation is incorrect as variance is ignored. +pub fn equal_up_to_regions( + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + src: Ty<'tcx>, + dest: Ty<'tcx>, +) -> bool { + struct LifetimeIgnoreRelation<'tcx> { + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + } + + impl TypeRelation<'tcx> for LifetimeIgnoreRelation<'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn param_env(&self) -> ty::ParamEnv<'tcx> { + self.param_env + } + + fn tag(&self) -> &'static str { + "librustc_mir::transform::validate" + } + + fn a_is_expected(&self) -> bool { + true + } + + fn relate_with_variance>( + &mut self, + _: ty::Variance, + a: &T, + b: &T, + ) -> RelateResult<'tcx, T> { + // Ignore variance, require types to be exactly the same. + self.relate(a, b) + } + + fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { + if a == b { + // Short-circuit. + return Ok(a); + } + ty::relate::super_relate_tys(self, a, b) + } + + fn regions( + &mut self, + a: ty::Region<'tcx>, + _b: ty::Region<'tcx>, + ) -> RelateResult<'tcx, ty::Region<'tcx>> { + // Ignore regions. + Ok(a) + } + + fn consts( + &mut self, + a: &'tcx ty::Const<'tcx>, + b: &'tcx ty::Const<'tcx>, + ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { + ty::relate::super_relate_consts(self, a, b) + } + + fn binders( + &mut self, + a: &ty::Binder, + b: &ty::Binder, + ) -> RelateResult<'tcx, ty::Binder> + where + T: Relate<'tcx>, + { + self.relate(a.skip_binder(), b.skip_binder())?; + Ok(a.clone()) + } + } + + // Instantiate and run relation. + let mut relator: LifetimeIgnoreRelation<'tcx> = LifetimeIgnoreRelation { tcx: tcx, param_env }; + relator.relate(&src, &dest).is_ok() +} + struct TypeChecker<'a, 'tcx> { when: &'a str, source: MirSource<'tcx>, @@ -108,80 +195,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // all normal lifetimes are erased, higher-ranked types with their // late-bound lifetimes are still around and can lead to type // differences. So we compare ignoring lifetimes. - struct LifetimeIgnoreRelation<'tcx> { - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - } - - impl TypeRelation<'tcx> for LifetimeIgnoreRelation<'tcx> { - fn tcx(&self) -> TyCtxt<'tcx> { - self.tcx - } - - fn param_env(&self) -> ty::ParamEnv<'tcx> { - self.param_env - } - - fn tag(&self) -> &'static str { - "librustc_mir::transform::validate" - } - - fn a_is_expected(&self) -> bool { - true - } - - fn relate_with_variance>( - &mut self, - _: ty::Variance, - a: &T, - b: &T, - ) -> RelateResult<'tcx, T> { - // Ignore variance, require types to be exactly the same. - self.relate(a, b) - } - - fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { - if a == b { - // Short-circuit. - return Ok(a); - } - ty::relate::super_relate_tys(self, a, b) - } - - fn regions( - &mut self, - a: ty::Region<'tcx>, - _b: ty::Region<'tcx>, - ) -> RelateResult<'tcx, ty::Region<'tcx>> { - // Ignore regions. - Ok(a) - } - - fn consts( - &mut self, - a: &'tcx ty::Const<'tcx>, - b: &'tcx ty::Const<'tcx>, - ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { - ty::relate::super_relate_consts(self, a, b) - } - - fn binders( - &mut self, - a: &ty::Binder, - b: &ty::Binder, - ) -> RelateResult<'tcx, ty::Binder> - where - T: Relate<'tcx>, - { - self.relate(a.skip_binder(), b.skip_binder())?; - Ok(a.clone()) - } - } - - // Instantiate and run relation. - let mut relator: LifetimeIgnoreRelation<'tcx> = - LifetimeIgnoreRelation { tcx: self.tcx, param_env }; - relator.relate(&src, &dest).is_ok() + equal_up_to_regions(self.tcx, param_env, src, dest) } } From 5e5ae8b0875ce70b1a729bef442e753bb3d2502f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Jun 2020 10:26:29 +0200 Subject: [PATCH 13/28] expand a comment --- src/librustc_mir/interpret/eval_context.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index b673738cec580..56a9355650e22 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -226,7 +226,9 @@ pub(super) fn mir_assign_valid_types<'tcx>( dest: TyAndLayout<'tcx>, ) -> bool { if src.ty == dest.ty { - // Equal types, all is good. + // Equal types, all is good. Layout will also be equal. + // (Enum variants would be an exception here as they have the type of the enum but different layout. + // However, those are never the type of an assignment.) return true; } if src.layout != dest.layout { From a593728fc753f85df575b54aadc64c3fd2c06d11 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Jun 2020 11:07:39 +0200 Subject: [PATCH 14/28] make layout check a mere sanity check --- src/librustc_mir/interpret/eval_context.rs | 25 ++++++---------------- src/librustc_mir/transform/validate.rs | 10 +++++---- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 56a9355650e22..25860c43add41 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -225,28 +225,17 @@ pub(super) fn mir_assign_valid_types<'tcx>( src: TyAndLayout<'tcx>, dest: TyAndLayout<'tcx>, ) -> bool { - if src.ty == dest.ty { - // Equal types, all is good. Layout will also be equal. - // (Enum variants would be an exception here as they have the type of the enum but different layout. - // However, those are never the type of an assignment.) - return true; - } - if src.layout != dest.layout { - // Layout differs, definitely not equal. - // We do this here because Miri would *do the wrong thing* if we allowed layout-changing - // assignments. - return false; - } - // Type-changing assignments can happen when subtyping is used. While // all normal lifetimes are erased, higher-ranked types with their // late-bound lifetimes are still around and can lead to type // differences. So we compare ignoring lifetimes. - // - // Note that this is not fully correct (FIXME): - // lifetimes in invariant positions could matter (e.g. through associated types). - // We rely on the fact that layout was confirmed to be equal above. - equal_up_to_regions(tcx, param_env, src.ty, dest.ty) + if equal_up_to_regions(tcx, param_env, src.ty, dest.ty) { + // Make sure the layout is equal, too -- just to be safe. Miri really needs layout equality. + assert_eq!(src.layout, dest.layout); + true + } else { + false + } } /// Use the already known layout if given (but sanity check in debug mode), diff --git a/src/librustc_mir/transform/validate.rs b/src/librustc_mir/transform/validate.rs index 5f0edd64c47e1..a293751d5b276 100644 --- a/src/librustc_mir/transform/validate.rs +++ b/src/librustc_mir/transform/validate.rs @@ -44,6 +44,11 @@ pub fn equal_up_to_regions( src: Ty<'tcx>, dest: Ty<'tcx>, ) -> bool { + // Fast path. + if src == dest { + return true; + } + struct LifetimeIgnoreRelation<'tcx> { tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -176,6 +181,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { /// Check if src can be assigned into dest. /// This is not precise, it will accept some incorrect assignments. fn mir_assign_valid_types(&self, src: Ty<'tcx>, dest: Ty<'tcx>) -> bool { + // Fast path before we normalize. if src == dest { // Equal types, all is good. return true; @@ -186,10 +192,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { let param_env = self.param_env.with_reveal_all(); let src = self.tcx.normalize_erasing_regions(param_env, src); let dest = self.tcx.normalize_erasing_regions(param_env, dest); - // It's worth checking equality again. - if src == dest { - return true; - } // Type-changing assignments can happen when subtyping is used. While // all normal lifetimes are erased, higher-ranked types with their From 35911eeb93bae9585b6881bb3d6620df3e6a38ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Jun 2020 09:03:01 +0200 Subject: [PATCH 15/28] reduce sanity check in debug mode --- src/librustc_mir/interpret/eval_context.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 25860c43add41..ec792d5b22414 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -230,8 +230,14 @@ pub(super) fn mir_assign_valid_types<'tcx>( // late-bound lifetimes are still around and can lead to type // differences. So we compare ignoring lifetimes. if equal_up_to_regions(tcx, param_env, src.ty, dest.ty) { - // Make sure the layout is equal, too -- just to be safe. Miri really needs layout equality. - assert_eq!(src.layout, dest.layout); + // Make sure the layout is equal, too -- just to be safe. Miri really + // needs layout equality. For performance reason we skip this check when + // the types are equal. Equal types *can* have different layouts when + // enum downcast is involved (as enum variants carry the type of the + // enum), but those should never occur in assignments. + if cfg!(debug_assertions) || src.ty != dest.ty { + assert_eq!(src.layout, dest.layout); + } true } else { false From 5e28eb580ff48a84fe6f49bff31c4c022f243ac9 Mon Sep 17 00:00:00 2001 From: Nell Shamrell Date: Thu, 18 Jun 2020 10:30:47 -0700 Subject: [PATCH 16/28] Adds a clearer message for when the async keyword is missing from a function Signed-off-by: Nell Shamrell --- src/libcore/future/future.rs | 1 + src/test/ui/async-await/async-error-span.rs | 2 +- src/test/ui/async-await/async-error-span.stderr | 5 +++-- src/test/ui/async-await/issue-70594.rs | 2 +- src/test/ui/async-await/issue-70594.stderr | 5 +++-- src/test/ui/async-await/issues/issue-62009-1.stderr | 5 +++-- src/test/ui/issues-71798.rs | 2 +- src/test/ui/issues-71798.stderr | 5 +++-- ...ssed-as-arg-where-it-should-have-been-called.stderr | 10 ++++++---- 9 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index 00a171e6b5fb1..abf461338d80a 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -27,6 +27,7 @@ use crate::task::{Context, Poll}; #[must_use = "futures do nothing unless you `.await` or poll them"] #[stable(feature = "futures_api", since = "1.36.0")] #[lang = "future_trait"] +#[rustc_on_unimplemented(label = "`{Self}` is not a future", message = "`{Self}` is not a future")] pub trait Future { /// The type of value produced on completion. #[stable(feature = "futures_api", since = "1.36.0")] diff --git a/src/test/ui/async-await/async-error-span.rs b/src/test/ui/async-await/async-error-span.rs index cf10ebfeca939..86d459bf084b1 100644 --- a/src/test/ui/async-await/async-error-span.rs +++ b/src/test/ui/async-await/async-error-span.rs @@ -5,7 +5,7 @@ use std::future::Future; fn get_future() -> impl Future { -//~^ ERROR the trait bound `(): std::future::Future` is not satisfied +//~^ ERROR `()` is not a future panic!() } diff --git a/src/test/ui/async-await/async-error-span.stderr b/src/test/ui/async-await/async-error-span.stderr index 4054e739c483d..9523f040aa8cd 100644 --- a/src/test/ui/async-await/async-error-span.stderr +++ b/src/test/ui/async-await/async-error-span.stderr @@ -1,12 +1,13 @@ -error[E0277]: the trait bound `(): std::future::Future` is not satisfied +error[E0277]: `()` is not a future --> $DIR/async-error-span.rs:7:20 | LL | fn get_future() -> impl Future { - | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::future::Future` is not implemented for `()` + | ^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not a future LL | LL | panic!() | -------- this returned value is of type `!` | + = help: the trait `std::future::Future` is not implemented for `()` = note: the return type of a function must have a statically known size error[E0698]: type inside `async fn` body must be known in this context diff --git a/src/test/ui/async-await/issue-70594.rs b/src/test/ui/async-await/issue-70594.rs index e78231a68512d..34d12db8806dc 100644 --- a/src/test/ui/async-await/issue-70594.rs +++ b/src/test/ui/async-await/issue-70594.rs @@ -6,7 +6,7 @@ async fn fun() { //~| error: `.await` is not allowed in a `const` //~| error: `loop` is not allowed in a `const` //~| error: `.await` is not allowed in a `const` - //~| error: the trait bound `(): std::future::Future` is not satisfied + //~| error: `()` is not a future } fn main() {} diff --git a/src/test/ui/async-await/issue-70594.stderr b/src/test/ui/async-await/issue-70594.stderr index 496ca506c60f2..1b7abe317222d 100644 --- a/src/test/ui/async-await/issue-70594.stderr +++ b/src/test/ui/async-await/issue-70594.stderr @@ -27,12 +27,13 @@ error[E0744]: `.await` is not allowed in a `const` LL | [1; ().await]; | ^^^^^^^^ -error[E0277]: the trait bound `(): std::future::Future` is not satisfied +error[E0277]: `()` is not a future --> $DIR/issue-70594.rs:4:9 | LL | [1; ().await]; - | ^^^^^^^^ the trait `std::future::Future` is not implemented for `()` + | ^^^^^^^^ `()` is not a future | + = help: the trait `std::future::Future` is not implemented for `()` = note: required by `std::future::Future::poll` error: aborting due to 5 previous errors diff --git a/src/test/ui/async-await/issues/issue-62009-1.stderr b/src/test/ui/async-await/issues/issue-62009-1.stderr index ec4e9e397a81e..e3ba74a03c898 100644 --- a/src/test/ui/async-await/issues/issue-62009-1.stderr +++ b/src/test/ui/async-await/issues/issue-62009-1.stderr @@ -27,12 +27,13 @@ LL | fn main() { LL | (|_| 2333).await; | ^^^^^^^^^^^^^^^^ only allowed inside `async` functions and blocks -error[E0277]: the trait bound `[closure@$DIR/issue-62009-1.rs:12:5: 12:15]: std::future::Future` is not satisfied +error[E0277]: `[closure@$DIR/issue-62009-1.rs:12:5: 12:15]` is not a future --> $DIR/issue-62009-1.rs:12:5 | LL | (|_| 2333).await; - | ^^^^^^^^^^^^^^^^ the trait `std::future::Future` is not implemented for `[closure@$DIR/issue-62009-1.rs:12:5: 12:15]` + | ^^^^^^^^^^^^^^^^ `[closure@$DIR/issue-62009-1.rs:12:5: 12:15]` is not a future | + = help: the trait `std::future::Future` is not implemented for `[closure@$DIR/issue-62009-1.rs:12:5: 12:15]` = note: required by `std::future::Future::poll` error: aborting due to 4 previous errors diff --git a/src/test/ui/issues-71798.rs b/src/test/ui/issues-71798.rs index 08b10463d3927..fecba721ac9fd 100644 --- a/src/test/ui/issues-71798.rs +++ b/src/test/ui/issues-71798.rs @@ -1,5 +1,5 @@ fn test_ref(x: &u32) -> impl std::future::Future + '_ { - *x //~^ ERROR the trait bound `u32: std::future::Future` is not satisfied + *x //~^ ERROR `u32` is not a future } fn main() { diff --git a/src/test/ui/issues-71798.stderr b/src/test/ui/issues-71798.stderr index 85da87914e768..b3b29a7264131 100644 --- a/src/test/ui/issues-71798.stderr +++ b/src/test/ui/issues-71798.stderr @@ -4,14 +4,15 @@ error[E0425]: cannot find value `u` in this scope LL | let _ = test_ref & u; | ^ not found in this scope -error[E0277]: the trait bound `u32: std::future::Future` is not satisfied +error[E0277]: `u32` is not a future --> $DIR/issues-71798.rs:1:25 | LL | fn test_ref(x: &u32) -> impl std::future::Future + '_ { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::future::Future` is not implemented for `u32` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `u32` is not a future LL | *x | -- this returned value is of type `u32` | + = help: the trait `std::future::Future` is not implemented for `u32` = note: the return type of a function must have a statically known size error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.stderr b/src/test/ui/suggestions/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.stderr index 99ba4e2a646e5..11372494772d0 100644 --- a/src/test/ui/suggestions/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.stderr +++ b/src/test/ui/suggestions/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `fn() -> impl std::future::Future {foo}: std::future::Future` is not satisfied +error[E0277]: `fn() -> impl std::future::Future {foo}` is not a future --> $DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:10:9 | LL | async fn foo() {} @@ -8,14 +8,15 @@ LL | fn bar(f: impl Future) {} | ----------------- required by this bound in `bar` ... LL | bar(foo); - | ^^^ the trait `std::future::Future` is not implemented for `fn() -> impl std::future::Future {foo}` + | ^^^ `fn() -> impl std::future::Future {foo}` is not a future | + = help: the trait `std::future::Future` is not implemented for `fn() -> impl std::future::Future {foo}` help: use parentheses to call the function | LL | bar(foo()); | ^^ -error[E0277]: the trait bound `[closure@$DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:11:25: 11:36]: std::future::Future` is not satisfied +error[E0277]: `[closure@$DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:11:25: 11:36]` is not a future --> $DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:12:9 | LL | fn bar(f: impl Future) {} @@ -24,8 +25,9 @@ LL | fn bar(f: impl Future) {} LL | let async_closure = async || (); | -------- consider calling this closure LL | bar(async_closure); - | ^^^^^^^^^^^^^ the trait `std::future::Future` is not implemented for `[closure@$DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:11:25: 11:36]` + | ^^^^^^^^^^^^^ `[closure@$DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:11:25: 11:36]` is not a future | + = help: the trait `std::future::Future` is not implemented for `[closure@$DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:11:25: 11:36]` help: use parentheses to call the closure | LL | bar(async_closure()); From 49f6166ef7825a39e980c0ba0904073379bb01e6 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 25 Jun 2020 18:52:41 -0700 Subject: [PATCH 17/28] Prepare for LLVM 11 --- src/libprofiler_builtins/build.rs | 16 ++-- src/librustc_codegen_llvm/back/lto.rs | 5 +- src/librustc_codegen_llvm/llvm/ffi.rs | 16 +++- src/librustc_codegen_ssa/common.rs | 2 + src/rustllvm/PassWrapper.cpp | 119 ++++++++++++++++++++------ src/rustllvm/RustWrapper.cpp | 58 +++++++++---- src/test/codegen/sanitizer-recover.rs | 6 +- src/tools/compiletest/src/header.rs | 2 +- 8 files changed, 167 insertions(+), 57 deletions(-) diff --git a/src/libprofiler_builtins/build.rs b/src/libprofiler_builtins/build.rs index e23e2f2c1306f..bb7d59e113c08 100644 --- a/src/libprofiler_builtins/build.rs +++ b/src/libprofiler_builtins/build.rs @@ -24,6 +24,12 @@ fn main() { "InstrProfilingUtil.c", "InstrProfilingValue.c", "InstrProfilingWriter.c", + // This file was renamed in LLVM 10. + "InstrProfilingRuntime.cc", + "InstrProfilingRuntime.cpp", + // These files were added in LLVM 11. + "InstrProfilingInternal.c", + "InstrProfilingBiasVar.c", ]; if target.contains("msvc") { @@ -69,14 +75,12 @@ fn main() { let src_root = root.join("lib").join("profile"); for src in profile_sources { - cfg.file(src_root.join(src)); + let path = src_root.join(src); + if path.exists() { + cfg.file(path); + } } - // The file was renamed in LLVM 10. - let old_runtime_path = src_root.join("InstrProfilingRuntime.cc"); - let new_runtime_path = src_root.join("InstrProfilingRuntime.cpp"); - cfg.file(if old_runtime_path.exists() { old_runtime_path } else { new_runtime_path }); - cfg.include(root.join("include")); cfg.warnings(false); cfg.compile("profiler-rt"); diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index d3e3441b087c2..9764c9a102e8a 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -797,6 +797,7 @@ pub unsafe fn optimize_thin_module( kind: ModuleKind::Regular, }; { + let target = &*module.module_llvm.tm; let llmod = module.module_llvm.llmod(); save_temp_bitcode(&cgcx, &module, "thin-lto-input"); @@ -833,7 +834,7 @@ pub unsafe fn optimize_thin_module( { let _timer = cgcx.prof.generic_activity_with_arg("LLVM_thin_lto_rename", thin_module.name()); - if !llvm::LLVMRustPrepareThinLTORename(thin_module.shared.data.0, llmod) { + if !llvm::LLVMRustPrepareThinLTORename(thin_module.shared.data.0, llmod, target) { let msg = "failed to prepare thin LTO module"; return Err(write::llvm_err(&diag_handler, msg)); } @@ -865,7 +866,7 @@ pub unsafe fn optimize_thin_module( { let _timer = cgcx.prof.generic_activity_with_arg("LLVM_thin_lto_import", thin_module.name()); - if !llvm::LLVMRustPrepareThinLTOImport(thin_module.shared.data.0, llmod) { + if !llvm::LLVMRustPrepareThinLTOImport(thin_module.shared.data.0, llmod, target) { let msg = "failed to prepare thin LTO module"; return Err(write::llvm_err(&diag_handler, msg)); } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 8063d97aa73a9..7beb4fc897472 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -233,6 +233,8 @@ pub enum TypeKind { Metadata = 14, X86_MMX = 15, Token = 16, + ScalableVector = 17, + BFloat = 18, } impl TypeKind { @@ -255,6 +257,8 @@ impl TypeKind { TypeKind::Metadata => rustc_codegen_ssa::common::TypeKind::Metadata, TypeKind::X86_MMX => rustc_codegen_ssa::common::TypeKind::X86_MMX, TypeKind::Token => rustc_codegen_ssa::common::TypeKind::Token, + TypeKind::ScalableVector => rustc_codegen_ssa::common::TypeKind::ScalableVector, + TypeKind::BFloat => rustc_codegen_ssa::common::TypeKind::BFloat, } } } @@ -2141,10 +2145,18 @@ extern "C" { PreservedSymbols: *const *const c_char, PreservedSymbolsLen: c_uint, ) -> Option<&'static mut ThinLTOData>; - pub fn LLVMRustPrepareThinLTORename(Data: &ThinLTOData, Module: &Module) -> bool; + pub fn LLVMRustPrepareThinLTORename( + Data: &ThinLTOData, + Module: &Module, + Target: &TargetMachine, + ) -> bool; pub fn LLVMRustPrepareThinLTOResolveWeak(Data: &ThinLTOData, Module: &Module) -> bool; pub fn LLVMRustPrepareThinLTOInternalize(Data: &ThinLTOData, Module: &Module) -> bool; - pub fn LLVMRustPrepareThinLTOImport(Data: &ThinLTOData, Module: &Module) -> bool; + pub fn LLVMRustPrepareThinLTOImport( + Data: &ThinLTOData, + Module: &Module, + Target: &TargetMachine, + ) -> bool; pub fn LLVMRustGetThinLTOModuleImports( Data: *const ThinLTOData, ModuleNameCallback: ThinLTOModuleNameCallback, diff --git a/src/librustc_codegen_ssa/common.rs b/src/librustc_codegen_ssa/common.rs index 0d0321ec4ae5e..432b2f3bdc3c1 100644 --- a/src/librustc_codegen_ssa/common.rs +++ b/src/librustc_codegen_ssa/common.rs @@ -98,6 +98,8 @@ pub enum TypeKind { Metadata, X86_MMX, Token, + ScalableVector, + BFloat, } // FIXME(mw): Anything that is produced via DepGraph::with_task() must implement diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index 9bc111c26ba6b..41b14714842fd 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/src/rustllvm/PassWrapper.cpp @@ -49,8 +49,10 @@ typedef struct LLVMOpaqueTargetMachine *LLVMTargetMachineRef; DEFINE_STDCXX_CONVERSION_FUNCTIONS(Pass, LLVMPassRef) DEFINE_STDCXX_CONVERSION_FUNCTIONS(TargetMachine, LLVMTargetMachineRef) +#if LLVM_VERSION_LT(11, 0) DEFINE_STDCXX_CONVERSION_FUNCTIONS(PassManagerBuilder, LLVMPassManagerBuilderRef) +#endif extern "C" void LLVMInitializePasses() { PassRegistry &Registry = *PassRegistry::getPassRegistry(); @@ -343,17 +345,17 @@ enum class LLVMRustPassBuilderOptLevel { static PassBuilder::OptimizationLevel fromRust(LLVMRustPassBuilderOptLevel Level) { switch (Level) { case LLVMRustPassBuilderOptLevel::O0: - return PassBuilder::O0; + return PassBuilder::OptimizationLevel::O0; case LLVMRustPassBuilderOptLevel::O1: - return PassBuilder::O1; + return PassBuilder::OptimizationLevel::O1; case LLVMRustPassBuilderOptLevel::O2: - return PassBuilder::O2; + return PassBuilder::OptimizationLevel::O2; case LLVMRustPassBuilderOptLevel::O3: - return PassBuilder::O3; + return PassBuilder::OptimizationLevel::O3; case LLVMRustPassBuilderOptLevel::Os: - return PassBuilder::Os; + return PassBuilder::OptimizationLevel::Os; case LLVMRustPassBuilderOptLevel::Oz: - return PassBuilder::Oz; + return PassBuilder::OptimizationLevel::Oz; default: report_fatal_error("Bad PassBuilderOptLevel."); } @@ -796,8 +798,13 @@ LLVMRustOptimizeWithNewPassManager( // We manually collect pipeline callbacks so we can apply them at O0, where the // PassBuilder does not create a pipeline. std::vector> PipelineStartEPCallbacks; +#if LLVM_VERSION_GE(11, 0) + std::vector> + OptimizerLastEPCallbacks; +#else std::vector> OptimizerLastEPCallbacks; +#endif if (VerifyIR) { PipelineStartEPCallbacks.push_back([VerifyIR](ModulePassManager &MPM) { @@ -811,6 +818,14 @@ LLVMRustOptimizeWithNewPassManager( SanitizerOptions->SanitizeMemoryTrackOrigins, SanitizerOptions->SanitizeMemoryRecover, /*CompileKernel=*/false); +#if LLVM_VERSION_GE(11, 0) + OptimizerLastEPCallbacks.push_back( + [Options](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) { + MPM.addPass(MemorySanitizerPass(Options)); + MPM.addPass(createModuleToFunctionPassAdaptor(MemorySanitizerPass(Options))); + } + ); +#else #if LLVM_VERSION_GE(10, 0) PipelineStartEPCallbacks.push_back([Options](ModulePassManager &MPM) { MPM.addPass(MemorySanitizerPass(Options)); @@ -821,9 +836,18 @@ LLVMRustOptimizeWithNewPassManager( FPM.addPass(MemorySanitizerPass(Options)); } ); +#endif } if (SanitizerOptions->SanitizeThread) { +#if LLVM_VERSION_GE(11, 0) + OptimizerLastEPCallbacks.push_back( + [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) { + MPM.addPass(ThreadSanitizerPass()); + MPM.addPass(createModuleToFunctionPassAdaptor(ThreadSanitizerPass())); + } + ); +#else #if LLVM_VERSION_GE(10, 0) PipelineStartEPCallbacks.push_back([](ModulePassManager &MPM) { MPM.addPass(ThreadSanitizerPass()); @@ -834,9 +858,22 @@ LLVMRustOptimizeWithNewPassManager( FPM.addPass(ThreadSanitizerPass()); } ); +#endif } if (SanitizerOptions->SanitizeAddress) { +#if LLVM_VERSION_GE(11, 0) + OptimizerLastEPCallbacks.push_back( + [SanitizerOptions](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) { + MPM.addPass(RequireAnalysisPass()); + MPM.addPass(ModuleAddressSanitizerPass( + /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover)); + MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass( + /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover, + /*UseAfterScope=*/true))); + } + ); +#else PipelineStartEPCallbacks.push_back([&](ModulePassManager &MPM) { MPM.addPass(RequireAnalysisPass()); }); @@ -853,21 +890,27 @@ LLVMRustOptimizeWithNewPassManager( /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover)); } ); +#endif } } ModulePassManager MPM(DebugPassManager); if (!NoPrepopulatePasses) { - if (OptLevel == PassBuilder::O0) { + if (OptLevel == PassBuilder::OptimizationLevel::O0) { for (const auto &C : PipelineStartEPCallbacks) C(MPM); +#if LLVM_VERSION_GE(11, 0) + for (const auto &C : OptimizerLastEPCallbacks) + C(MPM, OptLevel); +#else if (!OptimizerLastEPCallbacks.empty()) { FunctionPassManager FPM(DebugPassManager); for (const auto &C : OptimizerLastEPCallbacks) C(FPM, OptLevel); MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); } +#endif MPM.addPass(AlwaysInlinerPass(EmitLifetimeMarkers)); @@ -892,12 +935,17 @@ LLVMRustOptimizeWithNewPassManager( break; case LLVMRustOptStage::PreLinkThinLTO: MPM = PB.buildThinLTOPreLinkDefaultPipeline(OptLevel, DebugPassManager); +#if LLVM_VERSION_GE(11, 0) + for (const auto &C : OptimizerLastEPCallbacks) + C(MPM, OptLevel); +#else if (!OptimizerLastEPCallbacks.empty()) { FunctionPassManager FPM(DebugPassManager); for (const auto &C : OptimizerLastEPCallbacks) C(FPM, OptLevel); MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); } +#endif break; case LLVMRustOptStage::PreLinkFatLTO: MPM = PB.buildLTOPreLinkDefaultPipeline(OptLevel, DebugPassManager); @@ -994,10 +1042,10 @@ class RustAssemblyAnnotationWriter : public AssemblyAnnotationWriter { const Value *Value; if (const CallInst *CI = dyn_cast(I)) { Name = "call"; - Value = CI->getCalledValue(); + Value = CI->getCalledOperand(); } else if (const InvokeInst* II = dyn_cast(I)) { Name = "invoke"; - Value = II->getCalledValue(); + Value = II->getCalledOperand(); } else { // Could demangle more operations, e. g. // `store %place, @function`. @@ -1335,10 +1383,33 @@ LLVMRustFreeThinLTOData(LLVMRustThinLTOData *Data) { // `ProcessThinLTOModule` function. Here they're split up into separate steps // so rustc can save off the intermediate bytecode between each step. +#if LLVM_VERSION_GE(11, 0) +static bool +clearDSOLocalOnDeclarations(Module &Mod, TargetMachine &TM) { + // When linking an ELF shared object, dso_local should be dropped. We + // conservatively do this for -fpic. + bool ClearDSOLocalOnDeclarations = + TM.getTargetTriple().isOSBinFormatELF() && + TM.getRelocationModel() != Reloc::Static && + Mod.getPIELevel() == PIELevel::Default; + return ClearDSOLocalOnDeclarations; +} +#endif + extern "C" bool -LLVMRustPrepareThinLTORename(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { +LLVMRustPrepareThinLTORename(const LLVMRustThinLTOData *Data, LLVMModuleRef M, + LLVMTargetMachineRef TM) { Module &Mod = *unwrap(M); - if (renameModuleForThinLTO(Mod, Data->Index)) { + TargetMachine &Target = *unwrap(TM); + +#if LLVM_VERSION_GE(11, 0) + bool ClearDSOLocal = clearDSOLocalOnDeclarations(Mod, Target); + bool error = renameModuleForThinLTO(Mod, Data->Index, ClearDSOLocal); +#else + bool error = renameModuleForThinLTO(Mod, Data->Index); +#endif + + if (error) { LLVMRustSetLastError("renameModuleForThinLTO failed"); return false; } @@ -1362,8 +1433,10 @@ LLVMRustPrepareThinLTOInternalize(const LLVMRustThinLTOData *Data, LLVMModuleRef } extern "C" bool -LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { +LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M, + LLVMTargetMachineRef TM) { Module &Mod = *unwrap(M); + TargetMachine &Target = *unwrap(TM); const auto &ImportList = Data->ImportLists.lookup(Mod.getModuleIdentifier()); auto Loader = [&](StringRef Identifier) { @@ -1399,7 +1472,12 @@ LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { return MOrErr; }; +#if LLVM_VERSION_GE(11, 0) + bool ClearDSOLocal = clearDSOLocalOnDeclarations(Mod, Target); + FunctionImporter Importer(Data->Index, Loader, ClearDSOLocal); +#else FunctionImporter Importer(Data->Index, Loader); +#endif Expected Result = Importer.importFunctions(Mod, ImportList); if (!Result) { LLVMRustSetLastError(toString(Result.takeError()).c_str()); @@ -1558,22 +1636,11 @@ LLVMRustThinLTOPatchDICompileUnit(LLVMModuleRef Mod, DICompileUnit *Unit) { } // Use LLVM's built-in `DebugInfoFinder` to find a bunch of debuginfo and - // process it recursively. Note that we specifically iterate over instructions - // to ensure we feed everything into it. + // process it recursively. Note that we used to specifically iterate over + // instructions to ensure we feed everything into it, but `processModule` + // started doing this the same way in LLVM 7 (commit d769eb36ab2b8). DebugInfoFinder Finder; Finder.processModule(*M); - for (Function &F : M->functions()) { - for (auto &FI : F) { - for (Instruction &BI : FI) { - if (auto Loc = BI.getDebugLoc()) - Finder.processLocation(*M, Loc); - if (auto DVI = dyn_cast(&BI)) - Finder.processValue(*M, DVI); - if (auto DDI = dyn_cast(&BI)) - Finder.processDeclare(*M, DDI); - } - } - } // After we've found all our debuginfo, rewrite all subprograms to point to // the same `DICompileUnit`. diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index cdb3a157eab97..063b6acc604ea 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -1,5 +1,4 @@ #include "rustllvm.h" -#include "llvm/IR/CallSite.h" #include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" @@ -214,50 +213,50 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { extern "C" void LLVMRustAddCallSiteAttribute(LLVMValueRef Instr, unsigned Index, LLVMRustAttribute RustAttr) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::get(Call->getContext(), fromRust(RustAttr)); - Call.addAttribute(Index, Attr); + Call->addAttribute(Index, Attr); } extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint32_t Bytes) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); AttrBuilder B; B.addAlignmentAttr(Bytes); - Call.setAttributes(Call.getAttributes().addAttributes( + Call->setAttributes(Call->getAttributes().addAttributes( Call->getContext(), Index, B)); } extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableAttr(Bytes); - Call.setAttributes(Call.getAttributes().addAttributes( + Call->setAttributes(Call->getAttributes().addAttributes( Call->getContext(), Index, B)); } extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableOrNullAttr(Bytes); - Call.setAttributes(Call.getAttributes().addAttributes( + Call->setAttributes(Call->getAttributes().addAttributes( Call->getContext(), Index, B)); } extern "C" void LLVMRustAddByValCallSiteAttr(LLVMValueRef Instr, unsigned Index, LLVMTypeRef Ty) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); #if LLVM_VERSION_GE(9, 0) Attribute Attr = Attribute::getWithByValType(Call->getContext(), unwrap(Ty)); #else Attribute Attr = Attribute::get(Call->getContext(), Attribute::ByVal); #endif - Call.addAttribute(Index, Attr); + Call->addAttribute(Index, Attr); } extern "C" void LLVMRustAddFunctionAttribute(LLVMValueRef Fn, unsigned Index, @@ -336,20 +335,24 @@ extern "C" void LLVMRustSetHasUnsafeAlgebra(LLVMValueRef V) { extern "C" LLVMValueRef LLVMRustBuildAtomicLoad(LLVMBuilderRef B, LLVMValueRef Source, const char *Name, LLVMAtomicOrdering Order) { - LoadInst *LI = new LoadInst(unwrap(Source)); + Value *Ptr = unwrap(Source); + Type *Ty = Ptr->getType()->getPointerElementType(); + LoadInst *LI = unwrap(B)->CreateLoad(Ty, Ptr, Name); LI->setAtomic(fromRust(Order)); - return wrap(unwrap(B)->Insert(LI, Name)); + return wrap(LI); } extern "C" LLVMValueRef LLVMRustBuildAtomicStore(LLVMBuilderRef B, LLVMValueRef V, LLVMValueRef Target, LLVMAtomicOrdering Order) { - StoreInst *SI = new StoreInst(unwrap(V), unwrap(Target)); + StoreInst *SI = unwrap(B)->CreateStore(unwrap(V), unwrap(Target)); SI->setAtomic(fromRust(Order)); - return wrap(unwrap(B)->Insert(SI)); + return wrap(SI); } +// FIXME: Use the C-API LLVMBuildAtomicCmpXchg and LLVMSetWeak +// once we raise our minimum support to LLVM 10. extern "C" LLVMValueRef LLVMRustBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Target, LLVMValueRef Old, LLVMValueRef Source, @@ -965,8 +968,14 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateUnionType( extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateTemplateTypeParameter( LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name, size_t NameLen, LLVMMetadataRef Ty) { +#if LLVM_VERSION_GE(11, 0) + bool IsDefault = false; // FIXME: should we ever set this true? + return wrap(Builder->createTemplateTypeParameter( + unwrapDI(Scope), StringRef(Name, NameLen), unwrapDI(Ty), IsDefault)); +#else return wrap(Builder->createTemplateTypeParameter( unwrapDI(Scope), StringRef(Name, NameLen), unwrapDI(Ty))); +#endif } extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateNameSpace( @@ -1227,12 +1236,23 @@ extern "C" LLVMTypeKind LLVMRustGetTypeKind(LLVMTypeRef Ty) { return LLVMArrayTypeKind; case Type::PointerTyID: return LLVMPointerTypeKind; +#if LLVM_VERSION_GE(11, 0) + case Type::FixedVectorTyID: + return LLVMVectorTypeKind; +#else case Type::VectorTyID: return LLVMVectorTypeKind; +#endif case Type::X86_MMXTyID: return LLVMX86_MMXTypeKind; case Type::TokenTyID: return LLVMTokenTypeKind; +#if LLVM_VERSION_GE(11, 0) + case Type::ScalableVectorTyID: + return LLVMScalableVectorTypeKind; + case Type::BFloatTyID: + return LLVMBFloatTypeKind; +#endif } report_fatal_error("Unhandled TypeID."); } @@ -1359,10 +1379,12 @@ extern "C" void LLVMRustFreeOperandBundleDef(OperandBundleDef *Bundle) { extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMValueRef Fn, LLVMValueRef *Args, unsigned NumArgs, OperandBundleDef *Bundle) { + Value *Callee = unwrap(Fn); + FunctionType *FTy = cast(Callee->getType()->getPointerElementType()); unsigned Len = Bundle ? 1 : 0; ArrayRef Bundles = makeArrayRef(Bundle, Len); return wrap(unwrap(B)->CreateCall( - unwrap(Fn), makeArrayRef(unwrap(Args), NumArgs), Bundles)); + FTy, Callee, makeArrayRef(unwrap(Args), NumArgs), Bundles)); } extern "C" LLVMValueRef LLVMRustGetInstrprofIncrementIntrinsic(LLVMModuleRef M) { @@ -1422,9 +1444,11 @@ LLVMRustBuildInvoke(LLVMBuilderRef B, LLVMValueRef Fn, LLVMValueRef *Args, unsigned NumArgs, LLVMBasicBlockRef Then, LLVMBasicBlockRef Catch, OperandBundleDef *Bundle, const char *Name) { + Value *Callee = unwrap(Fn); + FunctionType *FTy = cast(Callee->getType()->getPointerElementType()); unsigned Len = Bundle ? 1 : 0; ArrayRef Bundles = makeArrayRef(Bundle, Len); - return wrap(unwrap(B)->CreateInvoke(unwrap(Fn), unwrap(Then), unwrap(Catch), + return wrap(unwrap(B)->CreateInvoke(FTy, Callee, unwrap(Then), unwrap(Catch), makeArrayRef(unwrap(Args), NumArgs), Bundles, Name)); } diff --git a/src/test/codegen/sanitizer-recover.rs b/src/test/codegen/sanitizer-recover.rs index 719f219ce4ef1..433d32abd37c6 100644 --- a/src/test/codegen/sanitizer-recover.rs +++ b/src/test/codegen/sanitizer-recover.rs @@ -27,17 +27,17 @@ // ASAN: } // // MSAN-LABEL: define i32 @penguin( -// MSAN: call void @__msan_warning_noreturn() +// MSAN: call void @__msan_warning{{(_with_origin_noreturn\(i32 0\)|_noreturn\(\))}} // MSAN: unreachable // MSAN: } // // MSAN-RECOVER-LABEL: define i32 @penguin( -// MSAN-RECOVER: call void @__msan_warning() +// MSAN-RECOVER: call void @__msan_warning{{(_with_origin\(i32 0\)|\(\))}} // MSAN-RECOVER-NOT: unreachable // MSAN-RECOVER: } // // MSAN-RECOVER-LTO-LABEL: define i32 @penguin( -// MSAN-RECOVER-LTO: call void @__msan_warning() +// MSAN-RECOVER-LTO: call void @__msan_warning{{(_with_origin\(i32 0\)|\(\))}} // MSAN-RECOVER-LTO-NOT: unreachable // MSAN-RECOVER-LTO: } // diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 7d2c83881d13b..571e7a59113ad 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -263,7 +263,7 @@ impl EarlyProps { } fn version_to_int(version: &str) -> u32 { - let version_without_suffix = version.split('-').next().unwrap(); + let version_without_suffix = version.trim_end_matches("git").split('-').next().unwrap(); let components: Vec = version_without_suffix .split('.') .map(|s| s.parse().expect("Malformed version component")) From a6417b9c3822a08e27e0f556d76b880e26c0554d Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 26 Jun 2020 11:21:31 +0100 Subject: [PATCH 18/28] improper_ctypes: fix remaining `Reveal:All` This commit replaces the remaining uses of `ParamEnv::reveal_all` with `LateContext`'s `param_env` (normally `Reveal::UserFacing`). Signed-off-by: David Wood --- src/librustc_lint/types.rs | 128 +++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 62 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index cfafb86fbedb1..8b9402fcf623e 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -11,7 +11,7 @@ use rustc_index::vec::Idx; use rustc_middle::mir::interpret::{sign_extend, truncate}; use rustc_middle::ty::layout::{IntegerExt, SizeSkeleton}; use rustc_middle::ty::subst::SubstsRef; -use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, AdtKind, Ty, TypeFoldable}; use rustc_span::source_map; use rustc_span::symbol::sym; use rustc_span::{Span, DUMMY_SP}; @@ -524,78 +524,82 @@ enum FfiResult<'tcx> { FfiUnsafe { ty: Ty<'tcx>, reason: String, help: Option }, } -fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { - match ty.kind { - ty::FnPtr(_) => true, - ty::Ref(..) => true, - ty::Adt(field_def, substs) if field_def.repr.transparent() && !field_def.is_union() => { - for field in field_def.all_fields() { - let field_ty = - tcx.normalize_erasing_regions(ParamEnv::reveal_all(), field.ty(tcx, substs)); - if field_ty.is_zst(tcx, field.did) { - continue; - } +impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { + /// Is type known to be non-null? + fn ty_is_known_nonnull(&self, ty: Ty<'tcx>) -> bool { + match ty.kind { + ty::FnPtr(_) => true, + ty::Ref(..) => true, + ty::Adt(field_def, substs) if field_def.repr.transparent() && !field_def.is_union() => { + for field in field_def.all_fields() { + let field_ty = self.cx.tcx.normalize_erasing_regions( + self.cx.param_env, + field.ty(self.cx.tcx, substs), + ); + if field_ty.is_zst(self.cx.tcx, field.did) { + continue; + } - let attrs = tcx.get_attrs(field_def.did); - if attrs.iter().any(|a| a.check_name(sym::rustc_nonnull_optimization_guaranteed)) - || ty_is_known_nonnull(tcx, field_ty) - { - return true; + let attrs = self.cx.tcx.get_attrs(field_def.did); + if attrs + .iter() + .any(|a| a.check_name(sym::rustc_nonnull_optimization_guaranteed)) + || self.ty_is_known_nonnull(field_ty) + { + return true; + } } - } - false + false + } + _ => false, } - _ => false, } -} -/// Check if this enum can be safely exported based on the -/// "nullable pointer optimization". Currently restricted -/// to function pointers, references, core::num::NonZero*, -/// core::ptr::NonNull, and #[repr(transparent)] newtypes. -/// FIXME: This duplicates code in codegen. -fn is_repr_nullable_ptr<'tcx>( - tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, - ty_def: &'tcx ty::AdtDef, - substs: SubstsRef<'tcx>, -) -> bool { - if ty_def.variants.len() != 2 { - return false; - } + /// Check if this enum can be safely exported based on the "nullable pointer optimization". + /// Currently restricted to function pointers, references, `core::num::NonZero*`, + /// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes. + fn is_repr_nullable_ptr( + &self, + ty: Ty<'tcx>, + ty_def: &'tcx ty::AdtDef, + substs: SubstsRef<'tcx>, + ) -> bool { + if ty_def.variants.len() != 2 { + return false; + } - let get_variant_fields = |index| &ty_def.variants[VariantIdx::new(index)].fields; - let variant_fields = [get_variant_fields(0), get_variant_fields(1)]; - let fields = if variant_fields[0].is_empty() { - &variant_fields[1] - } else if variant_fields[1].is_empty() { - &variant_fields[0] - } else { - return false; - }; + let get_variant_fields = |index| &ty_def.variants[VariantIdx::new(index)].fields; + let variant_fields = [get_variant_fields(0), get_variant_fields(1)]; + let fields = if variant_fields[0].is_empty() { + &variant_fields[1] + } else if variant_fields[1].is_empty() { + &variant_fields[0] + } else { + return false; + }; - if fields.len() != 1 { - return false; - } + if fields.len() != 1 { + return false; + } - let field_ty = fields[0].ty(tcx, substs); - if !ty_is_known_nonnull(tcx, field_ty) { - return false; - } + let field_ty = fields[0].ty(self.cx.tcx, substs); + if !self.ty_is_known_nonnull(field_ty) { + return false; + } - // At this point, the field's type is known to be nonnull and the parent enum is Option-like. - // If the computed size for the field and the enum are different, the nonnull optimization isn't - // being applied (and we've got a problem somewhere). - let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, ParamEnv::reveal_all()).unwrap(); - if !compute_size_skeleton(ty).same_size(compute_size_skeleton(field_ty)) { - bug!("improper_ctypes: Option nonnull optimization not applied?"); - } + // At this point, the field's type is known to be nonnull and the parent enum is + // Option-like. If the computed size for the field and the enum are different, the non-null + // optimization isn't being applied (and we've got a problem somewhere). + let compute_size_skeleton = + |t| SizeSkeleton::compute(t, self.cx.tcx, self.cx.param_env).unwrap(); + if !compute_size_skeleton(ty).same_size(compute_size_skeleton(field_ty)) { + bug!("improper_ctypes: Option nonnull optimization not applied?"); + } - true -} + true + } -impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// Check if the type is array and emit an unsafe type lint. fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { if let ty::Array(..) = ty.kind { @@ -737,7 +741,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // discriminant. if !def.repr.c() && !def.repr.transparent() && def.repr.int.is_none() { // Special-case types like `Option`. - if !is_repr_nullable_ptr(cx, ty, def, substs) { + if !self.is_repr_nullable_ptr(ty, def, substs) { return FfiUnsafe { ty, reason: "enum has no representation hint".into(), From 3678e5c97e701c265ecada08cf6a6f52f8bac3cc Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 26 Jun 2020 13:18:16 +0100 Subject: [PATCH 19/28] errors: use `-Z terminal-width` in JSON emitter This commit makes the JSON emitter use `-Z terminal-width` in the "rendered" field of the JSON output. Signed-off-by: David Wood --- src/librustc_errors/json.rs | 15 ++++++++- src/librustc_session/session.rs | 16 +++++++--- src/librustdoc/core.rs | 11 +++++-- src/test/ui/terminal-width/flag-human.rs | 9 ++++++ src/test/ui/terminal-width/flag-human.stderr | 11 +++++++ src/test/ui/terminal-width/flag-json.rs | 9 ++++++ src/test/ui/terminal-width/flag-json.stderr | 32 ++++++++++++++++++++ 7 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/terminal-width/flag-human.rs create mode 100644 src/test/ui/terminal-width/flag-human.stderr create mode 100644 src/test/ui/terminal-width/flag-json.rs create mode 100644 src/test/ui/terminal-width/flag-json.stderr diff --git a/src/librustc_errors/json.rs b/src/librustc_errors/json.rs index 1382825922b0e..24186198fd2b1 100644 --- a/src/librustc_errors/json.rs +++ b/src/librustc_errors/json.rs @@ -36,6 +36,7 @@ pub struct JsonEmitter { pretty: bool, ui_testing: bool, json_rendered: HumanReadableErrorType, + terminal_width: Option, macro_backtrace: bool, } @@ -45,6 +46,7 @@ impl JsonEmitter { source_map: Lrc, pretty: bool, json_rendered: HumanReadableErrorType, + terminal_width: Option, macro_backtrace: bool, ) -> JsonEmitter { JsonEmitter { @@ -54,6 +56,7 @@ impl JsonEmitter { pretty, ui_testing: false, json_rendered, + terminal_width, macro_backtrace, } } @@ -61,6 +64,7 @@ impl JsonEmitter { pub fn basic( pretty: bool, json_rendered: HumanReadableErrorType, + terminal_width: Option, macro_backtrace: bool, ) -> JsonEmitter { let file_path_mapping = FilePathMapping::empty(); @@ -69,6 +73,7 @@ impl JsonEmitter { Lrc::new(SourceMap::new(file_path_mapping)), pretty, json_rendered, + terminal_width, macro_backtrace, ) } @@ -79,6 +84,7 @@ impl JsonEmitter { source_map: Lrc, pretty: bool, json_rendered: HumanReadableErrorType, + terminal_width: Option, macro_backtrace: bool, ) -> JsonEmitter { JsonEmitter { @@ -88,6 +94,7 @@ impl JsonEmitter { pretty, ui_testing: false, json_rendered, + terminal_width, macro_backtrace, } } @@ -247,7 +254,13 @@ impl Diagnostic { let buf = BufWriter::default(); let output = buf.clone(); je.json_rendered - .new_emitter(Box::new(buf), Some(je.sm.clone()), false, None, je.macro_backtrace) + .new_emitter( + Box::new(buf), + Some(je.sm.clone()), + false, + je.terminal_width, + je.macro_backtrace, + ) .ui_testing(je.ui_testing) .emit_diagnostic(diag); let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap(); diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 2ea312c42dc64..fcd5dab94a6c2 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -1061,8 +1061,15 @@ fn default_emitter( } } (config::ErrorOutputType::Json { pretty, json_rendered }, None) => Box::new( - JsonEmitter::stderr(Some(registry), source_map, pretty, json_rendered, macro_backtrace) - .ui_testing(sopts.debugging_opts.ui_testing), + JsonEmitter::stderr( + Some(registry), + source_map, + pretty, + json_rendered, + sopts.debugging_opts.terminal_width, + macro_backtrace, + ) + .ui_testing(sopts.debugging_opts.ui_testing), ), (config::ErrorOutputType::Json { pretty, json_rendered }, Some(dst)) => Box::new( JsonEmitter::new( @@ -1071,6 +1078,7 @@ fn default_emitter( source_map, pretty, json_rendered, + sopts.debugging_opts.terminal_width, macro_backtrace, ) .ui_testing(sopts.debugging_opts.ui_testing), @@ -1416,7 +1424,7 @@ pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! { Box::new(EmitterWriter::stderr(color_config, None, short, false, None, false)) } config::ErrorOutputType::Json { pretty, json_rendered } => { - Box::new(JsonEmitter::basic(pretty, json_rendered, false)) + Box::new(JsonEmitter::basic(pretty, json_rendered, None, false)) } }; let handler = rustc_errors::Handler::with_emitter(true, None, emitter); @@ -1431,7 +1439,7 @@ pub fn early_warn(output: config::ErrorOutputType, msg: &str) { Box::new(EmitterWriter::stderr(color_config, None, short, false, None, false)) } config::ErrorOutputType::Json { pretty, json_rendered } => { - Box::new(JsonEmitter::basic(pretty, json_rendered, false)) + Box::new(JsonEmitter::basic(pretty, json_rendered, None, false)) } }; let handler = rustc_errors::Handler::with_emitter(true, None, emitter); diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 1690b946bb625..5c80fdfc8cfcc 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -191,8 +191,15 @@ pub fn new_handler( Lrc::new(source_map::SourceMap::new(source_map::FilePathMapping::empty())) }); Box::new( - JsonEmitter::stderr(None, source_map, pretty, json_rendered, false) - .ui_testing(debugging_opts.ui_testing), + JsonEmitter::stderr( + None, + source_map, + pretty, + json_rendered, + debugging_opts.terminal_width, + false, + ) + .ui_testing(debugging_opts.ui_testing), ) } }; diff --git a/src/test/ui/terminal-width/flag-human.rs b/src/test/ui/terminal-width/flag-human.rs new file mode 100644 index 0000000000000..e445a84fd012e --- /dev/null +++ b/src/test/ui/terminal-width/flag-human.rs @@ -0,0 +1,9 @@ +// compile-flags: -Z terminal-width=20 + +// This test checks that `-Z terminal-width` effects the human error output by restricting it to an +// arbitrarily low value so that the effect is visible. + +fn main() { + let _: () = 42; + //~^ ERROR mismatched types +} diff --git a/src/test/ui/terminal-width/flag-human.stderr b/src/test/ui/terminal-width/flag-human.stderr new file mode 100644 index 0000000000000..393dcf2b82845 --- /dev/null +++ b/src/test/ui/terminal-width/flag-human.stderr @@ -0,0 +1,11 @@ +error[E0308]: mismatched types + --> $DIR/flag-human.rs:7:17 + | +LL | ..._: () = 42; + | -- ^^ expected `()`, found integer + | | + | expected due to this + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/terminal-width/flag-json.rs b/src/test/ui/terminal-width/flag-json.rs new file mode 100644 index 0000000000000..eabdc59ddedd5 --- /dev/null +++ b/src/test/ui/terminal-width/flag-json.rs @@ -0,0 +1,9 @@ +// compile-flags: -Z terminal-width=20 --error-format=json + +// This test checks that `-Z terminal-width` effects the JSON error output by restricting it to an +// arbitrarily low value so that the effect is visible. + +fn main() { + let _: () = 42; + //~^ ERROR mismatched types +} diff --git a/src/test/ui/terminal-width/flag-json.stderr b/src/test/ui/terminal-width/flag-json.stderr new file mode 100644 index 0000000000000..29730ccdd4ed7 --- /dev/null +++ b/src/test/ui/terminal-width/flag-json.stderr @@ -0,0 +1,32 @@ +{"message":"mismatched types","code":{"code":"E0308","explanation":"Expected type did not match the received type. + +Erroneous code example: + +```compile_fail,E0308 +let x: i32 = \"I am not a number!\"; +// ~~~ ~~~~~~~~~~~~~~~~~~~~ +// | | +// | initializing expression; +// | compiler infers type `&str` +// | +// type `i32` assigned to variable `x` +``` + +This error occurs when the compiler is unable to infer the concrete type of a +variable. It can occur in several cases, the most common being a mismatch +between two types: the type the author explicitly assigned, and the type the +compiler inferred. +"},"level":"error","spans":[{"file_name":"$DIR/flag-json.rs","byte_start":244,"byte_end":246,"line_start":7,"line_end":7,"column_start":17,"column_end":19,"is_primary":true,"text":[{"text":" let _: () = 42;","highlight_start":17,"highlight_end":19}],"label":"expected `()`, found integer","suggested_replacement":null,"suggestion_applicability":null,"expansion":null},{"file_name":"$DIR/flag-json.rs","byte_start":239,"byte_end":241,"line_start":7,"line_end":7,"column_start":12,"column_end":14,"is_primary":false,"text":[{"text":" let _: () = 42;","highlight_start":12,"highlight_end":14}],"label":"expected due to this","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error[E0308]: mismatched types + --> $DIR/flag-json.rs:7:17 + | +LL | ..._: () = 42; + | -- ^^ expected `()`, found integer + | | + | expected due to this + +"} +{"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error + +"} +{"message":"For more information about this error, try `rustc --explain E0308`.","code":null,"level":"failure-note","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0308`. +"} From 36ac08e2643dc5cc035031007a8a36f4c87d3543 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 11 Jun 2020 13:42:22 -0400 Subject: [PATCH 20/28] Make `fn_arg_names` return `Ident` instead of symbol Also, implement this query for the local crate, not just foreign crates. --- src/librustc_metadata/rmeta/decoder.rs | 4 ++-- src/librustc_metadata/rmeta/encoder.rs | 16 +++++----------- src/librustc_metadata/rmeta/mod.rs | 4 ++-- src/librustc_middle/hir/map/mod.rs | 9 ++++++++- src/librustc_middle/hir/mod.rs | 20 ++++++++++++++++---- src/librustc_middle/query/mod.rs | 2 +- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs index 25e57aa77acd0..2254d553337d5 100644 --- a/src/librustc_metadata/rmeta/decoder.rs +++ b/src/librustc_metadata/rmeta/decoder.rs @@ -1339,13 +1339,13 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { } } - fn get_fn_param_names(&self, tcx: TyCtxt<'tcx>, id: DefIndex) -> &'tcx [Symbol] { + fn get_fn_param_names(&self, tcx: TyCtxt<'tcx>, id: DefIndex) -> &'tcx [Ident] { let param_names = match self.kind(id) { EntryKind::Fn(data) | EntryKind::ForeignFn(data) => data.decode(self).param_names, EntryKind::AssocFn(data) => data.decode(self).fn_data.param_names, _ => Lazy::empty(), }; - tcx.arena.alloc_from_iter(param_names.decode(self)) + tcx.arena.alloc_from_iter(param_names.decode((self, tcx))) } fn exported_symbols( diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index cdc8b5e90a642..d01c767e2bc04 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -30,7 +30,7 @@ use rustc_middle::ty::{self, SymbolName, Ty, TyCtxt}; use rustc_serialize::{opaque, Encodable, Encoder, SpecializedEncoder, UseSpecializedEncodable}; use rustc_session::config::CrateType; use rustc_span::source_map::Spanned; -use rustc_span::symbol::{kw, sym, Ident, Symbol}; +use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::{self, ExternalSource, FileName, SourceFile, Span}; use rustc_target::abi::VariantIdx; use std::hash::Hash; @@ -1004,18 +1004,12 @@ impl EncodeContext<'tcx> { } } - fn encode_fn_param_names_for_body(&mut self, body_id: hir::BodyId) -> Lazy<[Symbol]> { - self.tcx.dep_graph.with_ignore(|| { - let body = self.tcx.hir().body(body_id); - self.lazy(body.params.iter().map(|arg| match arg.pat.kind { - hir::PatKind::Binding(_, _, ident, _) => ident.name, - _ => kw::Invalid, - })) - }) + fn encode_fn_param_names_for_body(&mut self, body_id: hir::BodyId) -> Lazy<[Ident]> { + self.tcx.dep_graph.with_ignore(|| self.lazy(self.tcx.hir().body_param_names(body_id))) } - fn encode_fn_param_names(&mut self, param_names: &[Ident]) -> Lazy<[Symbol]> { - self.lazy(param_names.iter().map(|ident| ident.name)) + fn encode_fn_param_names(&mut self, param_names: &[Ident]) -> Lazy<[Ident]> { + self.lazy(param_names.iter()) } fn encode_optimized_mir(&mut self, def_id: LocalDefId) { diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index 0edea63f922d6..381e7ee115e17 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -19,7 +19,7 @@ use rustc_serialize::opaque::Encoder; use rustc_session::config::SymbolManglingVersion; use rustc_session::CrateDisambiguator; use rustc_span::edition::Edition; -use rustc_span::symbol::Symbol; +use rustc_span::symbol::{Ident, Symbol}; use rustc_span::{self, Span}; use rustc_target::spec::{PanicStrategy, TargetTriple}; @@ -327,7 +327,7 @@ struct ModData { struct FnData { asyncness: hir::IsAsync, constness: hir::Constness, - param_names: Lazy<[Symbol]>, + param_names: Lazy<[Ident]>, } #[derive(RustcEncodable, RustcDecodable)] diff --git a/src/librustc_middle/hir/map/mod.rs b/src/librustc_middle/hir/map/mod.rs index 3a4fc581f5f26..d60d24aa9aed5 100644 --- a/src/librustc_middle/hir/map/mod.rs +++ b/src/librustc_middle/hir/map/mod.rs @@ -14,7 +14,7 @@ use rustc_hir::*; use rustc_index::vec::IndexVec; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::Spanned; -use rustc_span::symbol::{kw, Symbol}; +use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::Span; use rustc_target::spec::abi::Abi; @@ -374,6 +374,13 @@ impl<'hir> Map<'hir> { }) } + pub fn body_param_names(&self, id: BodyId) -> impl Iterator + 'hir { + self.body(id).params.iter().map(|arg| match arg.pat.kind { + PatKind::Binding(_, _, ident, _) => ident, + _ => Ident::new(kw::Invalid, rustc_span::DUMMY_SP), + }) + } + /// Returns the `BodyOwnerKind` of this `LocalDefId`. /// /// Panics if `LocalDefId` does not have an associated body. diff --git a/src/librustc_middle/hir/mod.rs b/src/librustc_middle/hir/mod.rs index 1e3676496ce39..e152d11c081a1 100644 --- a/src/librustc_middle/hir/mod.rs +++ b/src/librustc_middle/hir/mod.rs @@ -12,10 +12,7 @@ use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; -use rustc_hir::Body; -use rustc_hir::HirId; -use rustc_hir::ItemLocalId; -use rustc_hir::Node; +use rustc_hir::*; use rustc_index::vec::IndexVec; pub struct Owner<'tcx> { @@ -79,5 +76,20 @@ pub fn provide(providers: &mut Providers<'_>) { }; providers.hir_owner = |tcx, id| tcx.index_hir(LOCAL_CRATE).map[id].signature; providers.hir_owner_nodes = |tcx, id| tcx.index_hir(LOCAL_CRATE).map[id].with_bodies.as_deref(); + providers.fn_arg_names = |tcx, id| { + let hir = tcx.hir(); + let hir_id = hir.as_local_hir_id(id.expect_local()); + if let Some(body_id) = hir.maybe_body_owned_by(hir_id) { + tcx.arena.alloc_from_iter(hir.body_param_names(body_id)) + } else if let Node::TraitItem(&TraitItem { + kind: TraitItemKind::Fn(_, TraitFn::Required(idents)), + .. + }) = hir.get(hir_id) + { + tcx.arena.alloc_slice(idents) + } else { + span_bug!(hir.span(hir_id), "fn_arg_names: unexpected item {:?}", id); + } + }; map::provide(providers); } diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 2f51b98085b4e..f10f38dc935a8 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -729,7 +729,7 @@ rustc_queries! { } Other { - query fn_arg_names(def_id: DefId) -> &'tcx [Symbol] { + query fn_arg_names(def_id: DefId) -> &'tcx [rustc_span::symbol::Ident] { desc { |tcx| "looking up function parameter names for `{}`", tcx.def_path_str(def_id) } } /// Gets the rendered value of the specified constant or associated constant. From fa6a61c68930b390407d73e0ba71f2af5555f0f0 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 11 Jun 2020 13:48:46 -0400 Subject: [PATCH 21/28] Explain move errors that occur due to method calls involving `self` This is a re-attempt of #72389 (which was reverted in #73594) Instead of using `ExpnKind::Desugaring` to represent operators, this PR checks the lang item directly. --- src/librustc_ast_lowering/expr.rs | 17 +- src/librustc_hir/lang_items.rs | 80 ++++++--- .../infer/error_reporting/need_type_info.rs | 2 +- src/librustc_middle/lint.rs | 2 +- .../diagnostics/conflict_errors.rs | 72 +++++++- .../diagnostics/explain_borrow.rs | 4 +- .../borrow_check/diagnostics/mod.rs | 157 ++++++++++++++--- .../borrow_check/diagnostics/move_errors.rs | 2 +- .../diagnostics/mutability_errors.rs | 2 +- src/librustc_mir/borrow_check/mod.rs | 6 + src/librustc_mir/transform/const_prop.rs | 1 + src/librustc_passes/lang_items.rs | 6 +- src/librustc_span/hygiene.rs | 11 +- src/librustc_span/lib.rs | 3 +- src/test/ui/binop/binop-consume-args.stderr | 70 ++++++-- src/test/ui/binop/binop-move-semantics.stderr | 21 ++- .../borrowck/borrowck-unboxed-closures.stderr | 7 +- .../ui/closure_context/issue-42065.stderr | 7 +- src/test/ui/codemap_tests/tab_3.stderr | 8 +- src/test/ui/issues/issue-12127.stderr | 7 +- src/test/ui/issues/issue-33941.rs | 1 + src/test/ui/issues/issue-33941.stderr | 12 +- src/test/ui/issues/issue-34721.stderr | 9 +- src/test/ui/issues/issue-61108.stderr | 8 +- src/test/ui/issues/issue-64559.stderr | 8 +- src/test/ui/moves/move-fn-self-receiver.rs | 74 ++++++++ .../ui/moves/move-fn-self-receiver.stderr | 158 ++++++++++++++++++ ...moves-based-on-type-access-to-field.stderr | 8 +- .../ui/moves/moves-based-on-type-exprs.stderr | 16 +- .../ui/once-cant-call-twice-on-heap.stderr | 7 +- ...ed-closures-infer-fnonce-call-twice.stderr | 7 +- ...osures-infer-fnonce-move-call-twice.stderr | 7 +- src/test/ui/unop-move-semantics.stderr | 7 +- .../unsized-locals/borrow-after-move.stderr | 8 +- src/test/ui/unsized-locals/double-move.stderr | 8 +- .../use-after-move-self-based-on-type.stderr | 8 +- src/test/ui/use/use-after-move-self.stderr | 8 +- src/test/ui/walk-struct-literal-with.stderr | 8 +- 38 files changed, 745 insertions(+), 102 deletions(-) create mode 100644 src/test/ui/moves/move-fn-self-receiver.rs create mode 100644 src/test/ui/moves/move-fn-self-receiver.stderr diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index d2c4478ccfeb6..90a3a5ec64e0e 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -9,7 +9,7 @@ use rustc_data_structures::thin_vec::ThinVec; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned}; +use rustc_span::source_map::{respan, DesugaringKind, ForLoopLoc, Span, Spanned}; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_target::asm; use std::collections::hash_map::Entry; @@ -1361,9 +1361,14 @@ impl<'hir> LoweringContext<'_, 'hir> { body: &Block, opt_label: Option