From 358dc1d8c2e10eceaf3c04d532bbde73b0dd4bb7 Mon Sep 17 00:00:00 2001 From: Nathan West Date: Thu, 28 May 2020 15:02:48 -0400 Subject: [PATCH 01/25] Added io forwarding methods to the stdio structs --- src/libstd/io/stdio.rs | 83 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index b65b150d2c3a1..cce9a0dc7a43b 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -96,7 +96,20 @@ impl Read for StdinRaw { unsafe fn initializer(&self) -> Initializer { Initializer::nop() } + + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + self.0.read_to_end(buf) + } + + fn read_to_string(&mut self, buf: &mut String) -> io::Result { + self.0.read_to_string(buf) + } + + fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { + self.0.read_exact(buf) + } } + impl Write for StdoutRaw { fn write(&mut self, buf: &[u8]) -> io::Result { self.0.write(buf) @@ -114,7 +127,20 @@ impl Write for StdoutRaw { fn flush(&mut self) -> io::Result<()> { self.0.flush() } + + fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + self.0.write_all(buf) + } + + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + self.0.write_all_vectored(bufs) + } + + fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { + self.0.write_fmt(fmt) + } } + impl Write for StderrRaw { fn write(&mut self, buf: &[u8]) -> io::Result { self.0.write(buf) @@ -132,6 +158,18 @@ impl Write for StderrRaw { fn flush(&mut self) -> io::Result<()> { self.0.flush() } + + fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + self.0.write_all(buf) + } + + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + self.0.write_all_vectored(bufs) + } + + fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { + self.0.write_fmt(fmt) + } } enum Maybe { @@ -420,6 +458,18 @@ impl Read for StdinLock<'_> { unsafe fn initializer(&self) -> Initializer { Initializer::nop() } + + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + self.inner.read_to_end(buf) + } + + fn read_to_string(&mut self, buf: &mut String) -> io::Result { + self.inner.read_to_string(buf) + } + + fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { + self.inner.read_exact(buf) + } } #[stable(feature = "rust1", since = "1.0.0")] @@ -427,9 +477,18 @@ impl BufRead for StdinLock<'_> { fn fill_buf(&mut self) -> io::Result<&[u8]> { self.inner.fill_buf() } + fn consume(&mut self, n: usize) { self.inner.consume(n) } + + fn read_until(&mut self, byte: u8, buf: &mut Vec) -> io::Result { + self.inner.read_until(byte, buf) + } + + fn read_line(&mut self, buf: &mut String) -> io::Result { + self.inner.read_line(buf) + } } #[stable(feature = "std_debug", since = "1.16.0")] @@ -596,6 +655,9 @@ impl Write for Stdout { fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> { self.lock().write_fmt(args) } + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + self.lock().write_all_vectored(bufs) + } } #[stable(feature = "rust1", since = "1.0.0")] impl Write for StdoutLock<'_> { @@ -612,6 +674,15 @@ impl Write for StdoutLock<'_> { fn flush(&mut self) -> io::Result<()> { self.inner.borrow_mut().flush() } + fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + self.inner.borrow_mut().write_all(buf) + } + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + self.inner.borrow_mut().write_all_vectored(bufs) + } + fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { + self.inner.borrow_mut().write_fmt(fmt) + } } #[stable(feature = "std_debug", since = "1.16.0")] @@ -770,6 +841,9 @@ impl Write for Stderr { fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> { self.lock().write_fmt(args) } + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + self.lock().write_all_vectored(bufs) + } } #[stable(feature = "rust1", since = "1.0.0")] impl Write for StderrLock<'_> { @@ -786,6 +860,15 @@ impl Write for StderrLock<'_> { fn flush(&mut self) -> io::Result<()> { self.inner.borrow_mut().flush() } + fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + self.inner.borrow_mut().write_all(buf) + } + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + self.inner.borrow_mut().write_all_vectored(bufs) + } + fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { + self.inner.borrow_mut().write_fmt(fmt) + } } #[stable(feature = "std_debug", since = "1.16.0")] From 93cbad6ed5f6a40bdd1e8cee6a9b1a39f17ab166 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 17:44:33 +0200 Subject: [PATCH 02/25] 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 03/25] 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 04/25] 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 b60cefee0addb02b5bd146893d358bb52bc829e2 Mon Sep 17 00:00:00 2001 From: Nathan West Date: Wed, 17 Jun 2020 17:56:25 -0400 Subject: [PATCH 05/25] Removed write_fmt forwarding, to fix recursive borrow issues --- src/libstd/io/stdio.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index cce9a0dc7a43b..b41f0c2f82d0d 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -652,9 +652,6 @@ impl Write for Stdout { fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { self.lock().write_all(buf) } - fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> { - self.lock().write_fmt(args) - } fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { self.lock().write_all_vectored(bufs) } @@ -680,9 +677,6 @@ impl Write for StdoutLock<'_> { fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { self.inner.borrow_mut().write_all_vectored(bufs) } - fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { - self.inner.borrow_mut().write_fmt(fmt) - } } #[stable(feature = "std_debug", since = "1.16.0")] @@ -838,9 +832,6 @@ impl Write for Stderr { fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { self.lock().write_all(buf) } - fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> { - self.lock().write_fmt(args) - } fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { self.lock().write_all_vectored(bufs) } @@ -866,9 +857,6 @@ impl Write for StderrLock<'_> { fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { self.inner.borrow_mut().write_all_vectored(bufs) } - fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { - self.inner.borrow_mut().write_fmt(fmt) - } } #[stable(feature = "std_debug", since = "1.16.0")] From 14d385bedeeec7fcb48f4c9bb881b1cdae011da0 Mon Sep 17 00:00:00 2001 From: Nathan West Date: Wed, 17 Jun 2020 19:48:51 -0400 Subject: [PATCH 06/25] Restore some write_fmts --- src/libstd/io/stdio.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index b41f0c2f82d0d..d6b7ad6254a8c 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -655,6 +655,9 @@ impl Write for Stdout { fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { self.lock().write_all_vectored(bufs) } + fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> { + self.lock().write_fmt(args) + } } #[stable(feature = "rust1", since = "1.0.0")] impl Write for StdoutLock<'_> { @@ -835,6 +838,9 @@ impl Write for Stderr { fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { self.lock().write_all_vectored(bufs) } + fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> { + self.lock().write_fmt(args) + } } #[stable(feature = "rust1", since = "1.0.0")] impl Write for StderrLock<'_> { From 810f309ff30fe7a75917f9e5359074dc991b4590 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 30 May 2020 23:46:21 +0200 Subject: [PATCH 07/25] 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 08/25] 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 09/25] 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 10/25] 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 11/25] 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 12/25] 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 13/25] 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 14/25] 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 15/25] 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 16/25] 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 17/25] 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 18/25] 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 19/25] 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 20/25] 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 df88972f8ce9ddbebec6d551810f7127fe25d2a3 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 27 Jun 2020 01:16:23 +0300 Subject: [PATCH 21/25] Update psm version This new version includes a fix for building on aarch64 windows. --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8cbf6cf6869ea..97b4e79a03c9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2663,9 +2663,9 @@ dependencies = [ [[package]] name = "psm" -version = "0.1.8" +version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "659ecfea2142a458893bb7673134bad50b752fea932349c213d6a23874ce3aa7" +checksum = "092d385624a084892d07374caa7b0994956692cf40650419a1f1a787a8d229cf" dependencies = [ "cc", ] From 9308860a7b6dbeda34d2c116fd4109abe9969aca Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Sat, 27 Jun 2020 21:38:51 +0900 Subject: [PATCH 22/25] fix typo in self-profile.md --- src/doc/unstable-book/src/compiler-flags/self-profile.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/unstable-book/src/compiler-flags/self-profile.md b/src/doc/unstable-book/src/compiler-flags/self-profile.md index 6de1c774f7cd7..7305141a42714 100644 --- a/src/doc/unstable-book/src/compiler-flags/self-profile.md +++ b/src/doc/unstable-book/src/compiler-flags/self-profile.md @@ -13,7 +13,7 @@ For example: First, run a compilation session and provide the `-Zself-profile` flag: ```console -$ rustc --crate-name foo -Zself-profile` +$ rustc --crate-name foo -Zself-profile ``` This will generate three files in the working directory such as: From 4c14f9d110478edcb2d0c3e1cda73937fc3b3d6e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 27 Jun 2020 15:23:01 +0200 Subject: [PATCH 23/25] Forward Hash::write_iN to Hash::write_uN --- src/libcore/hash/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libcore/hash/mod.rs b/src/libcore/hash/mod.rs index d80101753cbef..6abe19dc155d1 100644 --- a/src/libcore/hash/mod.rs +++ b/src/libcore/hash/mod.rs @@ -333,31 +333,31 @@ pub trait Hasher { #[inline] #[stable(feature = "hasher_write", since = "1.3.0")] fn write_i16(&mut self, i: i16) { - self.write(&i.to_ne_bytes()) + self.write_u16(i as u16) } /// Writes a single `i32` into this hasher. #[inline] #[stable(feature = "hasher_write", since = "1.3.0")] fn write_i32(&mut self, i: i32) { - self.write(&i.to_ne_bytes()) + self.write_u32(i as u32) } /// Writes a single `i64` into this hasher. #[inline] #[stable(feature = "hasher_write", since = "1.3.0")] fn write_i64(&mut self, i: i64) { - self.write(&i.to_ne_bytes()) + self.write_u64(i as u64) } /// Writes a single `i128` into this hasher. #[inline] #[stable(feature = "i128", since = "1.26.0")] fn write_i128(&mut self, i: i128) { - self.write(&i.to_ne_bytes()) + self.write_u128(i as u128) } /// Writes a single `isize` into this hasher. #[inline] #[stable(feature = "hasher_write", since = "1.3.0")] fn write_isize(&mut self, i: isize) { - self.write(&i.to_ne_bytes()) + self.write_usize(i as usize) } } From d25d6c5bd8c211a5e606b2ac37bbecb9be1ac12b Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 27 Jun 2020 18:10:58 +0200 Subject: [PATCH 24/25] Update the documentation to point to open instead of is_file and is_dir --- src/libstd/fs.rs | 24 ++++++++++++++---------- src/libstd/path.rs | 2 +- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index a9c481dfb809f..23bd8f6498b4a 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1033,14 +1033,16 @@ 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. + /// When the goal is simply to read from (or write to) the source, the most + /// reliable way to test the source can be read (or written to) is to open + /// it. Only using `is_file` can break workflows like `diff <( prog_a )` on + /// a Unix-like system for example. See [`File::open`] or + /// [`OpenOptions::open`] for more information. /// /// [`is_dir`]: struct.Metadata.html#method.is_dir /// [`symlink_metadata`]: fn.symlink_metadata.html + /// [`File::open`]: struct.File.html#method.open + /// [`OpenOptions::open`]: struct.OpenOptions.html#method.open /// /// # Examples /// @@ -1313,14 +1315,16 @@ 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. + /// When the goal is simply to read from (or write to) the source, the most + /// reliable way to test the source can be read (or written to) is to open + /// it. Only using `is_file` can break workflows like `diff <( prog_a )` on + /// a Unix-like system for example. See [`File::open`] or + /// [`OpenOptions::open`] for more information. /// /// [`is_dir`]: struct.FileType.html#method.is_dir /// [`is_symlink`]: struct.FileType.html#method.is_symlink + /// [`File::open`]: struct.File.html#method.open + /// [`OpenOptions::open`]: struct.OpenOptions.html#method.open /// /// # Examples /// diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 35f6b5838a92a..fa54b2063b424 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2506,7 +2506,7 @@ impl Path { /// check errors, call [`fs::metadata`] and handle its Result. Then call /// [`fs::Metadata::is_file`] if it was Ok. /// - /// Note that the explanation about using `!is_dir` instead of `is_file` + /// Note that the explanation about using `open` instead of `is_file` /// that is present in the [`fs::Metadata`] documentation also applies here. /// /// [`fs::metadata`]: ../../std/fs/fn.metadata.html From 8e8c54aa3a8d92d8443ec4596754d14b2d196899 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 27 Jun 2020 22:59:47 +0200 Subject: [PATCH 25/25] Added the parapgrah to path::Path::is_file too --- src/libstd/path.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index fa54b2063b424..f14a9ff72f62f 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2506,12 +2506,17 @@ impl Path { /// check errors, call [`fs::metadata`] and handle its Result. Then call /// [`fs::Metadata::is_file`] if it was Ok. /// - /// Note that the explanation about using `open` instead of `is_file` - /// that is present in the [`fs::Metadata`] documentation also applies here. + /// When the goal is simply to read from (or write to) the source, the most + /// reliable way to test the source can be read (or written to) is to open + /// it. Only using `is_file` can break workflows like `diff <( prog_a )` on + /// a Unix-like system for example. See [`File::open`] or + /// [`OpenOptions::open`] for more information. /// /// [`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 + /// [`File::open`]: ../../std/fs/struct.File.html#method.open + /// [`OpenOptions::open`]: ../../std/fs/struct.OpenOptions.html#method.open #[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)