From 358dc1d8c2e10eceaf3c04d532bbde73b0dd4bb7 Mon Sep 17 00:00:00 2001 From: Nathan West Date: Thu, 28 May 2020 15:02:48 -0400 Subject: [PATCH 01/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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 3678e5c97e701c265ecada08cf6a6f52f8bac3cc Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 26 Jun 2020 13:18:16 +0100 Subject: [PATCH 20/29] 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 a4e7b4798405917560a224b6e9f2eab07524e0b7 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sat, 27 Jun 2020 13:09:54 +0200 Subject: [PATCH 21/29] use LocalDefId in module checking --- src/librustc_middle/hir/map/mod.rs | 4 ++-- src/librustc_middle/query/mod.rs | 26 ++++++++++++------------- src/librustc_passes/check_attr.rs | 4 ++-- src/librustc_passes/check_const.rs | 4 ++-- src/librustc_passes/hir_id_validator.rs | 2 +- src/librustc_passes/intrinsicck.rs | 4 ++-- src/librustc_passes/liveness.rs | 16 +++++++-------- src/librustc_passes/loops.rs | 4 ++-- src/librustc_passes/stability.rs | 4 ++-- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/collect.rs | 2 +- src/librustc_typeck/impl_wf_check.rs | 6 +++--- 12 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/librustc_middle/hir/map/mod.rs b/src/librustc_middle/hir/map/mod.rs index 3a4fc581f5f26..b883ce5f54da6 100644 --- a/src/librustc_middle/hir/map/mod.rs +++ b/src/librustc_middle/hir/map/mod.rs @@ -451,11 +451,11 @@ impl<'hir> Map<'hir> { } } - pub fn visit_item_likes_in_module(&self, module: DefId, visitor: &mut V) + pub fn visit_item_likes_in_module(&self, module: LocalDefId, visitor: &mut V) where V: ItemLikeVisitor<'hir>, { - let module = self.tcx.hir_module_items(module.expect_local()); + let module = self.tcx.hir_module_items(module); for id in &module.items { visitor.visit_item(self.expect_item(*id)); diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 2f51b98085b4e..9d639d3fa6c1d 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -15,11 +15,11 @@ use rustc_query_system::query::QueryDescription; use rustc_span::symbol::Symbol; use std::borrow::Cow; -fn describe_as_module(def_id: DefId, tcx: TyCtxt<'_>) -> String { +fn describe_as_module(def_id: LocalDefId, tcx: TyCtxt<'_>) -> String { if def_id.is_top_level_module() { "top-level module".to_string() } else { - format!("module `{}`", tcx.def_path_str(def_id)) + format!("module `{}`", tcx.def_path_str(def_id.to_def_id())) } } @@ -473,49 +473,49 @@ rustc_queries! { Other { query lint_mod(key: LocalDefId) -> () { - desc { |tcx| "linting {}", describe_as_module(key.to_def_id(), tcx) } + desc { |tcx| "linting {}", describe_as_module(key, tcx) } } /// Checks the attributes in the module. - query check_mod_attrs(key: DefId) -> () { + query check_mod_attrs(key: LocalDefId) -> () { desc { |tcx| "checking attributes in {}", describe_as_module(key, tcx) } } - query check_mod_unstable_api_usage(key: DefId) -> () { + query check_mod_unstable_api_usage(key: LocalDefId) -> () { desc { |tcx| "checking for unstable API usage in {}", describe_as_module(key, tcx) } } /// Checks the const bodies in the module for illegal operations (e.g. `if` or `loop`). - query check_mod_const_bodies(key: DefId) -> () { + query check_mod_const_bodies(key: LocalDefId) -> () { desc { |tcx| "checking consts in {}", describe_as_module(key, tcx) } } /// Checks the loops in the module. - query check_mod_loops(key: DefId) -> () { + query check_mod_loops(key: LocalDefId) -> () { desc { |tcx| "checking loops in {}", describe_as_module(key, tcx) } } - query check_mod_item_types(key: DefId) -> () { + query check_mod_item_types(key: LocalDefId) -> () { desc { |tcx| "checking item types in {}", describe_as_module(key, tcx) } } query check_mod_privacy(key: LocalDefId) -> () { - desc { |tcx| "checking privacy in {}", describe_as_module(key.to_def_id(), tcx) } + desc { |tcx| "checking privacy in {}", describe_as_module(key, tcx) } } - query check_mod_intrinsics(key: DefId) -> () { + query check_mod_intrinsics(key: LocalDefId) -> () { desc { |tcx| "checking intrinsics in {}", describe_as_module(key, tcx) } } - query check_mod_liveness(key: DefId) -> () { + query check_mod_liveness(key: LocalDefId) -> () { desc { |tcx| "checking liveness of variables in {}", describe_as_module(key, tcx) } } - query check_mod_impl_wf(key: DefId) -> () { + query check_mod_impl_wf(key: LocalDefId) -> () { desc { |tcx| "checking that impls are well-formed in {}", describe_as_module(key, tcx) } } - query collect_mod_item_types(key: DefId) -> () { + query collect_mod_item_types(key: LocalDefId) -> () { desc { |tcx| "collecting item types in {}", describe_as_module(key, tcx) } } diff --git a/src/librustc_passes/check_attr.rs b/src/librustc_passes/check_attr.rs index 80681c143750f..87fba6b46ac59 100644 --- a/src/librustc_passes/check_attr.rs +++ b/src/librustc_passes/check_attr.rs @@ -12,7 +12,7 @@ use rustc_ast::ast::{Attribute, NestedMetaItem}; use rustc_ast::attr; use rustc_errors::struct_span_err; use rustc_hir as hir; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::{self, HirId, Item, ItemKind, TraitItem}; use rustc_hir::{MethodKind, Target}; @@ -461,7 +461,7 @@ fn is_c_like_enum(item: &Item<'_>) -> bool { } } -fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir() .visit_item_likes_in_module(module_def_id, &mut CheckAttrVisitor { tcx }.as_deep_visitor()); } diff --git a/src/librustc_passes/check_const.rs b/src/librustc_passes/check_const.rs index 94f9c619a3a26..90a076eeded7e 100644 --- a/src/librustc_passes/check_const.rs +++ b/src/librustc_passes/check_const.rs @@ -9,7 +9,7 @@ use rustc_errors::struct_span_err; use rustc_hir as hir; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_middle::hir::map::Map; use rustc_middle::ty::query::Providers; @@ -62,7 +62,7 @@ impl NonConstExpr { } } -fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let mut vis = CheckConstVisitor::new(tcx); tcx.hir().visit_item_likes_in_module(module_def_id, &mut vis.as_deep_visitor()); } diff --git a/src/librustc_passes/hir_id_validator.rs b/src/librustc_passes/hir_id_validator.rs index 80dfcd9c2417a..2edbc29b7efb6 100644 --- a/src/librustc_passes/hir_id_validator.rs +++ b/src/librustc_passes/hir_id_validator.rs @@ -17,7 +17,7 @@ pub fn check_crate(tcx: TyCtxt<'_>) { par_iter(&hir_map.krate().modules).for_each(|(module_id, _)| { let local_def_id = hir_map.local_def_id(*module_id); hir_map.visit_item_likes_in_module( - local_def_id.to_def_id(), + local_def_id, &mut OuterVisitor { hir_map, errors: &errors }, ); }); diff --git a/src/librustc_passes/intrinsicck.rs b/src/librustc_passes/intrinsicck.rs index c8666ba1fd078..683039df15ac6 100644 --- a/src/librustc_passes/intrinsicck.rs +++ b/src/librustc_passes/intrinsicck.rs @@ -2,7 +2,7 @@ use rustc_ast::ast::{FloatTy, InlineAsmTemplatePiece, IntTy, UintTy}; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_index::vec::Idx; use rustc_middle::ty::layout::{LayoutError, SizeSkeleton}; @@ -14,7 +14,7 @@ use rustc_target::abi::{Pointer, VariantIdx}; use rustc_target::asm::{InlineAsmRegOrRegClass, InlineAsmType}; use rustc_target::spec::abi::Abi::RustIntrinsic; -fn check_mod_intrinsics(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_intrinsics(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module(module_def_id, &mut ItemVisitor { tcx }.as_deep_visitor()); } diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index ff5dabd5418c9..798c6b8925bbf 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -89,7 +89,7 @@ use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::*; -use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{Expr, HirId, HirIdMap, HirIdSet, Node}; use rustc_middle::hir::map::Map; @@ -172,7 +172,7 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> { } } -fn check_mod_liveness(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_liveness(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module( module_def_id, &mut IrMaps::new(tcx, module_def_id).as_deep_visitor(), @@ -248,7 +248,7 @@ enum VarKind { struct IrMaps<'tcx> { tcx: TyCtxt<'tcx>, - body_owner: DefId, + body_owner: LocalDefId, num_live_nodes: usize, num_vars: usize, live_node_map: HirIdMap, @@ -259,7 +259,7 @@ struct IrMaps<'tcx> { } impl IrMaps<'tcx> { - fn new(tcx: TyCtxt<'tcx>, body_owner: DefId) -> IrMaps<'tcx> { + fn new(tcx: TyCtxt<'tcx>, body_owner: LocalDefId) -> IrMaps<'tcx> { IrMaps { tcx, body_owner, @@ -349,7 +349,7 @@ fn visit_fn<'tcx>( // swap in a new set of IR maps for this function body: let def_id = ir.tcx.hir().local_def_id(id); - let mut fn_maps = IrMaps::new(ir.tcx, def_id.to_def_id()); + let mut fn_maps = IrMaps::new(ir.tcx, def_id); // Don't run unused pass for #[derive()] if let FnKind::Method(..) = fk { @@ -484,7 +484,7 @@ fn visit_expr<'tcx>(ir: &mut IrMaps<'tcx>, expr: &'tcx Expr<'tcx>) { } ir.set_captures(expr.hir_id, call_caps); let old_body_owner = ir.body_owner; - ir.body_owner = closure_def_id.to_def_id(); + ir.body_owner = closure_def_id; intravisit::walk_expr(ir, expr); ir.body_owner = old_body_owner; } @@ -937,7 +937,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { for (&var_hir_id, upvar) in upvars.iter().rev() { let upvar_id = ty::UpvarId { var_path: ty::UpvarPath { hir_id: var_hir_id }, - closure_expr_id: self.ir.body_owner.expect_local(), + closure_expr_id: self.ir.body_owner, }; match self.tables.upvar_capture(upvar_id) { ty::UpvarCapture::ByRef(_) => { @@ -1614,7 +1614,7 @@ impl<'tcx> Liveness<'_, 'tcx> { let var = self.variable(var_hir_id, upvar.span); let upvar_id = ty::UpvarId { var_path: ty::UpvarPath { hir_id: var_hir_id }, - closure_expr_id: self.ir.body_owner.expect_local(), + closure_expr_id: self.ir.body_owner, }; match self.tables.upvar_capture(upvar_id) { ty::UpvarCapture::ByValue => {} diff --git a/src/librustc_passes/loops.rs b/src/librustc_passes/loops.rs index 767a6909d31d4..d7012d4d711df 100644 --- a/src/librustc_passes/loops.rs +++ b/src/librustc_passes/loops.rs @@ -2,7 +2,7 @@ use Context::*; use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::{Destination, Movability, Node}; use rustc_middle::hir::map::Map; @@ -29,7 +29,7 @@ struct CheckLoopVisitor<'a, 'hir> { cx: Context, } -fn check_mod_loops(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_loops(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module( module_def_id, &mut CheckLoopVisitor { sess: &tcx.sess, hir_map: tcx.hir(), cx: Normal }.as_deep_visitor(), diff --git a/src/librustc_passes/stability.rs b/src/librustc_passes/stability.rs index 054748c09fc44..ad512c63352f1 100644 --- a/src/librustc_passes/stability.rs +++ b/src/librustc_passes/stability.rs @@ -7,7 +7,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; +use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::{Generics, HirId, Item, StructField, Variant}; use rustc_middle::hir::map::Map; @@ -472,7 +472,7 @@ fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> { /// Cross-references the feature names of unstable APIs with enabled /// features and possibly prints errors. -fn check_mod_unstable_api_usage(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_unstable_api_usage(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module(module_def_id, &mut Checker { tcx }.as_deep_visitor()); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0325782e69d51..7c4048ab22302 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -737,7 +737,7 @@ pub fn check_wf_new(tcx: TyCtxt<'_>) { tcx.hir().krate().par_visit_all_item_likes(&visit); } -fn check_mod_item_types(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_item_types(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckItemTypesVisitor { tcx }); } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 054165f2b0977..b486e3d3536c9 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -55,7 +55,7 @@ struct OnlySelfBounds(bool); /////////////////////////////////////////////////////////////////////////// // Main entry point -fn collect_mod_item_types(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn collect_mod_item_types(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module( module_def_id, &mut CollectItemTypesVisitor { tcx }.as_deep_visitor(), diff --git a/src/librustc_typeck/impl_wf_check.rs b/src/librustc_typeck/impl_wf_check.rs index 37d383db68ab6..77cd1b3de0106 100644 --- a/src/librustc_typeck/impl_wf_check.rs +++ b/src/librustc_typeck/impl_wf_check.rs @@ -14,7 +14,7 @@ use min_specialization::check_min_specialization; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::struct_span_err; use rustc_hir as hir; -use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::def_id::LocalDefId; use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; @@ -59,11 +59,11 @@ pub fn impl_wf_check(tcx: TyCtxt<'_>) { // but it's one that we must perform earlier than the rest of // WfCheck. for &module in tcx.hir().krate().modules.keys() { - tcx.ensure().check_mod_impl_wf(tcx.hir().local_def_id(module).to_def_id()); + tcx.ensure().check_mod_impl_wf(tcx.hir().local_def_id(module)); } } -fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: DefId) { +fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let min_specialization = tcx.features().min_specialization; tcx.hir() .visit_item_likes_in_module(module_def_id, &mut ImplWfCheck { tcx, min_specialization }); From 1875c797721a84b3a4cbb77d4e4bd0a367ff22c3 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sat, 27 Jun 2020 13:15:12 +0200 Subject: [PATCH 22/29] more LocalDefId in ty::context --- .../nice_region_error/static_impl_trait.rs | 3 ++- src/librustc_middle/ty/context.rs | 11 +++++++---- .../borrow_check/diagnostics/region_errors.rs | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs index 46dad81a099bb..d881a529cad38 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -26,7 +26,8 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { ); let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?; debug!("try_report_static_impl_trait: anon_reg_sup={:?}", anon_reg_sup); - let fn_returns = self.tcx().return_type_impl_or_dyn_traits(anon_reg_sup.def_id); + let fn_returns = + self.tcx().return_type_impl_or_dyn_traits(anon_reg_sup.def_id.expect_local()); if fn_returns.is_empty() { return None; } diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index df08e083d2cbb..fb89e0e41d94d 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -1436,8 +1436,11 @@ impl<'tcx> TyCtxt<'tcx> { } /// Given a `DefId` for an `fn`, return all the `dyn` and `impl` traits in its return type. - pub fn return_type_impl_or_dyn_traits(&self, scope_def_id: DefId) -> Vec<&'tcx hir::Ty<'tcx>> { - let hir_id = self.hir().as_local_hir_id(scope_def_id.expect_local()); + pub fn return_type_impl_or_dyn_traits( + &self, + scope_def_id: LocalDefId, + ) -> Vec<&'tcx hir::Ty<'tcx>> { + let hir_id = self.hir().as_local_hir_id(scope_def_id); let hir_output = match self.hir().get(hir_id) { Node::Item(hir::Item { kind: @@ -1480,9 +1483,9 @@ impl<'tcx> TyCtxt<'tcx> { v.0 } - pub fn return_type_impl_trait(&self, scope_def_id: DefId) -> Option<(Ty<'tcx>, Span)> { + pub fn return_type_impl_trait(&self, scope_def_id: LocalDefId) -> Option<(Ty<'tcx>, Span)> { // HACK: `type_of_def_id()` will fail on these (#55796), so return `None`. - let hir_id = self.hir().as_local_hir_id(scope_def_id.expect_local()); + let hir_id = self.hir().as_local_hir_id(scope_def_id); match self.hir().get(hir_id) { Node::Item(item) => { match item.kind { diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index f1923b9e81c66..d639a0f69d451 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -583,7 +583,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { .infcx .tcx .is_suitable_region(f) - .map(|r| r.def_id) + .map(|r| r.def_id.expect_local()) .map(|id| self.infcx.tcx.return_type_impl_trait(id)) .unwrap_or(None) { From 2d280a55a9820ab18b25b8080d30e6d0a864ec17 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sat, 27 Jun 2020 13:38:00 +0200 Subject: [PATCH 23/29] more LocalDefId cleanup --- .../nice_region_error/find_anon_type.rs | 41 +- .../nice_region_error/named_anon_conflict.rs | 3 +- .../nice_region_error/static_impl_trait.rs | 395 +++++++++--------- .../error_reporting/nice_region_error/util.rs | 11 +- src/librustc_middle/ty/context.rs | 20 +- .../borrow_check/diagnostics/region_errors.rs | 8 +- .../borrow_check/universal_regions.rs | 41 +- 7 files changed, 259 insertions(+), 260 deletions(-) diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs index 6677c0e59f63a..20617bb8bd8fc 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs @@ -28,30 +28,27 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { br: &ty::BoundRegion, ) -> Option<(&hir::Ty<'tcx>, &hir::FnDecl<'tcx>)> { if let Some(anon_reg) = self.tcx().is_suitable_region(region) { - let def_id = anon_reg.def_id; - if let Some(def_id) = def_id.as_local() { - let hir_id = self.tcx().hir().as_local_hir_id(def_id); - let fndecl = match self.tcx().hir().get(hir_id) { - Node::Item(&hir::Item { kind: hir::ItemKind::Fn(ref m, ..), .. }) - | Node::TraitItem(&hir::TraitItem { - kind: hir::TraitItemKind::Fn(ref m, ..), - .. - }) - | Node::ImplItem(&hir::ImplItem { - kind: hir::ImplItemKind::Fn(ref m, ..), - .. - }) => &m.decl, - _ => return None, - }; + let hir_id = self.tcx().hir().as_local_hir_id(anon_reg.def_id); + let fndecl = match self.tcx().hir().get(hir_id) { + Node::Item(&hir::Item { kind: hir::ItemKind::Fn(ref m, ..), .. }) + | Node::TraitItem(&hir::TraitItem { + kind: hir::TraitItemKind::Fn(ref m, ..), + .. + }) + | Node::ImplItem(&hir::ImplItem { + kind: hir::ImplItemKind::Fn(ref m, ..), .. + }) => &m.decl, + _ => return None, + }; - return fndecl - .inputs - .iter() - .find_map(|arg| self.find_component_for_bound_region(arg, br)) - .map(|ty| (ty, &**fndecl)); - } + fndecl + .inputs + .iter() + .find_map(|arg| self.find_component_for_bound_region(arg, br)) + .map(|ty| (ty, &**fndecl)) + } else { + None } - None } // This method creates a FindNestedTypeVisitor which returns the type corresponding diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs index 3012928a09854..72deba990b0b5 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs @@ -75,8 +75,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { } if let Some((_, fndecl)) = self.find_anon_type(anon, &br) { - let is_self_anon = self.is_self_anon(is_first, scope_def_id); - if is_self_anon { + if self.is_self_anon(is_first, scope_def_id) { return None; } diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs index d881a529cad38..b6e971feb0e5f 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -10,220 +10,217 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { /// Print the error message for lifetime errors when the return type is a static impl Trait. pub(super) fn try_report_static_impl_trait(&self) -> Option { debug!("try_report_static_impl_trait(error={:?})", self.error); - if let Some(ref error) = self.error { - if let RegionResolutionError::SubSupConflict( - _, - var_origin, - sub_origin, - sub_r, - sup_origin, - sup_r, - ) = error - { - debug!( - "try_report_static_impl_trait(var={:?}, sub={:?} {:?} sup={:?} {:?})", - var_origin, sub_origin, sub_r, sup_origin, sup_r + if let Some(RegionResolutionError::SubSupConflict( + _, + var_origin, + ref sub_origin, + sub_r, + ref sup_origin, + sup_r, + )) = self.error + { + debug!( + "try_report_static_impl_trait(var={:?}, sub={:?} {:?} sup={:?} {:?})", + var_origin, sub_origin, sub_r, sup_origin, sup_r + ); + let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?; + debug!("try_report_static_impl_trait: anon_reg_sup={:?}", anon_reg_sup); + let fn_returns = self.tcx().return_type_impl_or_dyn_traits(anon_reg_sup.def_id); + if fn_returns.is_empty() { + return None; + } + debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns); + if *sub_r == RegionKind::ReStatic { + let sp = var_origin.span(); + let return_sp = sub_origin.span(); + let param_info = self.find_param_with_region(sup_r, sub_r)?; + let (lifetime_name, lifetime) = if sup_r.has_name() { + (sup_r.to_string(), format!("lifetime `{}`", sup_r)) + } else { + ("'_".to_owned(), "an anonymous lifetime `'_`".to_string()) + }; + let mut err = struct_span_err!( + self.tcx().sess, + sp, + E0759, + "cannot infer an appropriate lifetime" ); - let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?; - debug!("try_report_static_impl_trait: anon_reg_sup={:?}", anon_reg_sup); - let fn_returns = - self.tcx().return_type_impl_or_dyn_traits(anon_reg_sup.def_id.expect_local()); - if fn_returns.is_empty() { - return None; - } - debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns); - if **sub_r == RegionKind::ReStatic { - let sp = var_origin.span(); - let return_sp = sub_origin.span(); - let param_info = self.find_param_with_region(sup_r, sub_r)?; - let (lifetime_name, lifetime) = if sup_r.has_name() { - (sup_r.to_string(), format!("lifetime `{}`", sup_r)) + err.span_label( + param_info.param_ty_span, + &format!("this data with {}...", lifetime), + ); + debug!("try_report_static_impl_trait: param_info={:?}", param_info); + + // We try to make the output have fewer overlapping spans if possible. + if (sp == sup_origin.span() || !return_sp.overlaps(sup_origin.span())) + && sup_origin.span() != return_sp + { + // FIXME: account for `async fn` like in `async-await/issues/issue-62097.rs` + + // Customize the spans and labels depending on their relative order so + // that split sentences flow correctly. + if sup_origin.span().overlaps(return_sp) && sp == sup_origin.span() { + // Avoid the following: + // + // error: cannot infer an appropriate lifetime + // --> $DIR/must_outlive_least_region_or_bound.rs:18:50 + // | + // LL | fn foo(x: &i32) -> Box { Box::new(x) } + // | ---- ---------^- + // + // and instead show: + // + // error: cannot infer an appropriate lifetime + // --> $DIR/must_outlive_least_region_or_bound.rs:18:50 + // | + // LL | fn foo(x: &i32) -> Box { Box::new(x) } + // | ---- ^ + err.span_label( + sup_origin.span(), + "...is captured here, requiring it to live as long as `'static`", + ); } else { - ("'_".to_owned(), "an anonymous lifetime `'_`".to_string()) - }; - let mut err = struct_span_err!( - self.tcx().sess, - sp, - E0759, - "cannot infer an appropriate lifetime" - ); + err.span_label(sup_origin.span(), "...is captured here..."); + if return_sp < sup_origin.span() { + err.span_note( + return_sp, + "...and is required to live as long as `'static` here", + ); + } else { + err.span_label( + return_sp, + "...and is required to live as long as `'static` here", + ); + } + } + } else { err.span_label( - param_info.param_ty_span, - &format!("this data with {}...", lifetime), + return_sp, + "...is captured and required to live as long as `'static` here", ); - debug!("try_report_static_impl_trait: param_info={:?}", param_info); + } - // We try to make the output have fewer overlapping spans if possible. - if (sp == sup_origin.span() || !return_sp.overlaps(sup_origin.span())) - && sup_origin.span() != return_sp - { - // FIXME: account for `async fn` like in `async-await/issues/issue-62097.rs` + // FIXME: account for the need of parens in `&(dyn Trait + '_)` + let consider = "consider changing the"; + let declare = "to declare that the"; + let arg = match param_info.param.pat.simple_ident() { + Some(simple_ident) => format!("argument `{}`", simple_ident), + None => "the argument".to_string(), + }; + let explicit = + format!("you can add an explicit `{}` lifetime bound", lifetime_name); + let explicit_static = + format!("explicit `'static` bound to the lifetime of {}", arg); + let captures = format!("captures data from {}", arg); + let add_static_bound = + "alternatively, add an explicit `'static` bound to this reference"; + let plus_lt = format!(" + {}", lifetime_name); + for fn_return in fn_returns { + if fn_return.span.desugaring_kind().is_some() { + // Skip `async` desugaring `impl Future`. + continue; + } + match fn_return.kind { + TyKind::OpaqueDef(item_id, _) => { + let item = self.tcx().hir().item(item_id.id); + let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind { + opaque + } else { + err.emit(); + return Some(ErrorReported); + }; - // Customize the spans and labels depending on their relative order so - // that split sentences flow correctly. - if sup_origin.span().overlaps(return_sp) && sp == sup_origin.span() { - // Avoid the following: - // - // error: cannot infer an appropriate lifetime - // --> $DIR/must_outlive_least_region_or_bound.rs:18:50 - // | - // LL | fn foo(x: &i32) -> Box { Box::new(x) } - // | ---- ---------^- - // - // and instead show: - // - // error: cannot infer an appropriate lifetime - // --> $DIR/must_outlive_least_region_or_bound.rs:18:50 - // | - // LL | fn foo(x: &i32) -> Box { Box::new(x) } - // | ---- ^ - err.span_label( - sup_origin.span(), - "...is captured here, requiring it to live as long as `'static`", - ); - } else { - err.span_label(sup_origin.span(), "...is captured here..."); - if return_sp < sup_origin.span() { - err.span_note( - return_sp, - "...and is required to live as long as `'static` here", + if let Some(span) = opaque + .bounds + .iter() + .filter_map(|arg| match arg { + GenericBound::Outlives(Lifetime { + name: LifetimeName::Static, + span, + .. + }) => Some(*span), + _ => None, + }) + .next() + { + err.span_suggestion_verbose( + span, + &format!("{} `impl Trait`'s {}", consider, explicit_static), + lifetime_name.clone(), + Applicability::MaybeIncorrect, ); + err.span_suggestion_verbose( + param_info.param_ty_span, + add_static_bound, + param_info.param_ty.to_string(), + Applicability::MaybeIncorrect, + ); + } else if let Some(_) = opaque + .bounds + .iter() + .filter_map(|arg| match arg { + GenericBound::Outlives(Lifetime { name, span, .. }) + if name.ident().to_string() == lifetime_name => + { + Some(*span) + } + _ => None, + }) + .next() + { } else { - err.span_label( - return_sp, - "...and is required to live as long as `'static` here", + err.span_suggestion_verbose( + fn_return.span.shrink_to_hi(), + &format!( + "{declare} `impl Trait` {captures}, {explicit}", + declare = declare, + captures = captures, + explicit = explicit, + ), + plus_lt.clone(), + Applicability::MaybeIncorrect, ); } } - } else { - err.span_label( - return_sp, - "...is captured and required to live as long as `'static` here", - ); - } - - // FIXME: account for the need of parens in `&(dyn Trait + '_)` - let consider = "consider changing the"; - let declare = "to declare that the"; - let arg = match param_info.param.pat.simple_ident() { - Some(simple_ident) => format!("argument `{}`", simple_ident), - None => "the argument".to_string(), - }; - let explicit = - format!("you can add an explicit `{}` lifetime bound", lifetime_name); - let explicit_static = - format!("explicit `'static` bound to the lifetime of {}", arg); - let captures = format!("captures data from {}", arg); - let add_static_bound = - "alternatively, add an explicit `'static` bound to this reference"; - let plus_lt = format!(" + {}", lifetime_name); - for fn_return in fn_returns { - if fn_return.span.desugaring_kind().is_some() { - // Skip `async` desugaring `impl Future`. - continue; - } - match fn_return.kind { - TyKind::OpaqueDef(item_id, _) => { - let item = self.tcx().hir().item(item_id.id); - let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind { - opaque - } else { - err.emit(); - return Some(ErrorReported); - }; - - if let Some(span) = opaque - .bounds - .iter() - .filter_map(|arg| match arg { - GenericBound::Outlives(Lifetime { - name: LifetimeName::Static, - span, - .. - }) => Some(*span), - _ => None, - }) - .next() - { - err.span_suggestion_verbose( - span, - &format!("{} `impl Trait`'s {}", consider, explicit_static), - lifetime_name.clone(), - Applicability::MaybeIncorrect, - ); - err.span_suggestion_verbose( - param_info.param_ty_span, - add_static_bound, - param_info.param_ty.to_string(), - Applicability::MaybeIncorrect, - ); - } else if let Some(_) = opaque - .bounds - .iter() - .filter_map(|arg| match arg { - GenericBound::Outlives(Lifetime { name, span, .. }) - if name.ident().to_string() == lifetime_name => - { - Some(*span) - } - _ => None, - }) - .next() - { - } else { - err.span_suggestion_verbose( - fn_return.span.shrink_to_hi(), - &format!( - "{declare} `impl Trait` {captures}, {explicit}", - declare = declare, - captures = captures, - explicit = explicit, - ), - plus_lt.clone(), - Applicability::MaybeIncorrect, - ); - } + TyKind::TraitObject(_, lt) => match lt.name { + LifetimeName::ImplicitObjectLifetimeDefault => { + err.span_suggestion_verbose( + fn_return.span.shrink_to_hi(), + &format!( + "{declare} trait object {captures}, {explicit}", + declare = declare, + captures = captures, + explicit = explicit, + ), + plus_lt.clone(), + Applicability::MaybeIncorrect, + ); + } + name if name.ident().to_string() != lifetime_name => { + // With this check we avoid suggesting redundant bounds. This + // would happen if there are nested impl/dyn traits and only + // one of them has the bound we'd suggest already there, like + // in `impl Foo + '_`. + err.span_suggestion_verbose( + lt.span, + &format!("{} trait object's {}", consider, explicit_static), + lifetime_name.clone(), + Applicability::MaybeIncorrect, + ); + err.span_suggestion_verbose( + param_info.param_ty_span, + add_static_bound, + param_info.param_ty.to_string(), + Applicability::MaybeIncorrect, + ); } - TyKind::TraitObject(_, lt) => match lt.name { - LifetimeName::ImplicitObjectLifetimeDefault => { - err.span_suggestion_verbose( - fn_return.span.shrink_to_hi(), - &format!( - "{declare} trait object {captures}, {explicit}", - declare = declare, - captures = captures, - explicit = explicit, - ), - plus_lt.clone(), - Applicability::MaybeIncorrect, - ); - } - name if name.ident().to_string() != lifetime_name => { - // With this check we avoid suggesting redundant bounds. This - // would happen if there are nested impl/dyn traits and only - // one of them has the bound we'd suggest already there, like - // in `impl Foo + '_`. - err.span_suggestion_verbose( - lt.span, - &format!("{} trait object's {}", consider, explicit_static), - lifetime_name.clone(), - Applicability::MaybeIncorrect, - ); - err.span_suggestion_verbose( - param_info.param_ty_span, - add_static_bound, - param_info.param_ty.to_string(), - Applicability::MaybeIncorrect, - ); - } - _ => {} - }, _ => {} - } + }, + _ => {} } - err.emit(); - return Some(ErrorReported); } + err.emit(); + return Some(ErrorReported); } } None diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/util.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/util.rs index 22b130cdf5ffe..fa999abb1a86c 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/util.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/util.rs @@ -3,7 +3,7 @@ use crate::infer::error_reporting::nice_region_error::NiceRegionError; use rustc_hir as hir; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::LocalDefId; use rustc_middle::ty::{self, DefIdTree, Region, Ty}; use rustc_span::Span; @@ -92,7 +92,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { // FIXME(#42703) - Need to handle certain cases here. pub(super) fn is_return_type_anon( &self, - scope_def_id: DefId, + scope_def_id: LocalDefId, br: ty::BoundRegion, decl: &hir::FnDecl<'_>, ) -> Option { @@ -112,9 +112,12 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { // corresponds to self and if yes, we display E0312. // FIXME(#42700) - Need to format self properly to // enable E0621 for it. - pub(super) fn is_self_anon(&self, is_first: bool, scope_def_id: DefId) -> bool { + pub(super) fn is_self_anon(&self, is_first: bool, scope_def_id: LocalDefId) -> bool { is_first - && self.tcx().opt_associated_item(scope_def_id).map(|i| i.fn_has_self_parameter) + && self + .tcx() + .opt_associated_item(scope_def_id.to_def_id()) + .map(|i| i.fn_has_self_parameter) == Some(true) } } diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index fb89e0e41d94d..44c8c1f6fdba4 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -873,8 +873,8 @@ impl<'tcx> CommonConsts<'tcx> { // conflict. #[derive(Debug)] pub struct FreeRegionInfo { - // def id corresponding to FreeRegion - pub def_id: DefId, + // `LocalDefId` corresponding to FreeRegion + pub def_id: LocalDefId, // the bound region corresponding to FreeRegion pub boundregion: ty::BoundRegion, // checks if bound region is in Impl Item @@ -1412,14 +1412,17 @@ impl<'tcx> TyCtxt<'tcx> { // Returns the `DefId` and the `BoundRegion` corresponding to the given region. pub fn is_suitable_region(&self, region: Region<'tcx>) -> Option { let (suitable_region_binding_scope, bound_region) = match *region { - ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region), - ty::ReEarlyBound(ref ebr) => { - (self.parent(ebr.def_id).unwrap(), ty::BoundRegion::BrNamed(ebr.def_id, ebr.name)) + ty::ReFree(ref free_region) => { + (free_region.scope.expect_local(), free_region.bound_region) } + ty::ReEarlyBound(ref ebr) => ( + self.parent(ebr.def_id).unwrap().expect_local(), + ty::BoundRegion::BrNamed(ebr.def_id, ebr.name), + ), _ => return None, // not a free region }; - let hir_id = self.hir().as_local_hir_id(suitable_region_binding_scope.expect_local()); + let hir_id = self.hir().as_local_hir_id(suitable_region_binding_scope); let is_impl_item = match self.hir().find(hir_id) { Some(Node::Item(..) | Node::TraitItem(..)) => false, Some(Node::ImplItem(..)) => { @@ -1515,8 +1518,9 @@ impl<'tcx> TyCtxt<'tcx> { } // Checks if the bound region is in Impl Item. - pub fn is_bound_region_in_impl_item(&self, suitable_region_binding_scope: DefId) -> bool { - let container_id = self.associated_item(suitable_region_binding_scope).container.id(); + pub fn is_bound_region_in_impl_item(&self, suitable_region_binding_scope: LocalDefId) -> bool { + let container_id = + self.associated_item(suitable_region_binding_scope.to_def_id()).container.id(); if self.impl_trait_ref(container_id).is_some() { // For now, we do not try to target impls of traits. This is // because this message is going to suggest that the user diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index d639a0f69d451..99b9788c20b05 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -579,11 +579,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { if let (Some(f), Some(ty::RegionKind::ReStatic)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { - if let Some((ty::TyS { kind: ty::Opaque(did, substs), .. }, _)) = self + if let Some((&ty::TyS { kind: ty::Opaque(did, substs), .. }, _)) = self .infcx .tcx .is_suitable_region(f) - .map(|r| r.def_id.expect_local()) + .map(|r| r.def_id) .map(|id| self.infcx.tcx.return_type_impl_trait(id)) .unwrap_or(None) { @@ -592,7 +592,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // // eg. check for `impl Trait + 'static` instead of `impl Trait`. let has_static_predicate = { - let predicates_of = self.infcx.tcx.predicates_of(*did); + let predicates_of = self.infcx.tcx.predicates_of(did); let bounds = predicates_of.instantiate(self.infcx.tcx, substs); let mut found = false; @@ -625,7 +625,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { diag.help(&format!("consider replacing `{}` with `{}`", fr_name, static_str)); } else { // Otherwise, we should suggest adding a constraint on the return type. - let span = self.infcx.tcx.def_span(*did); + let span = self.infcx.tcx.def_span(did); if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) { let suggestable_fr_name = if fr_name.was_named() { fr_name.to_string() diff --git a/src/librustc_mir/borrow_check/universal_regions.rs b/src/librustc_mir/borrow_check/universal_regions.rs index 3003f4639d9fa..7b292ee71f99d 100644 --- a/src/librustc_mir/borrow_check/universal_regions.rs +++ b/src/librustc_mir/borrow_check/universal_regions.rs @@ -232,8 +232,7 @@ impl<'tcx> UniversalRegions<'tcx> { ) -> Self { let tcx = infcx.tcx; let mir_hir_id = tcx.hir().as_local_hir_id(mir_def_id); - UniversalRegionsBuilder { infcx, mir_def_id: mir_def_id.to_def_id(), mir_hir_id, param_env } - .build() + UniversalRegionsBuilder { infcx, mir_def_id, mir_hir_id, param_env }.build() } /// Given a reference to a closure type, extracts all the values @@ -389,7 +388,7 @@ impl<'tcx> UniversalRegions<'tcx> { struct UniversalRegionsBuilder<'cx, 'tcx> { infcx: &'cx InferCtxt<'cx, 'tcx>, - mir_def_id: DefId, + mir_def_id: LocalDefId, mir_hir_id: HirId, param_env: ty::ParamEnv<'tcx>, } @@ -418,7 +417,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { let mut indices = self.compute_indices(fr_static, defining_ty); debug!("build: indices={:?}", indices); - let closure_base_def_id = self.infcx.tcx.closure_base_def_id(self.mir_def_id); + let closure_base_def_id = self.infcx.tcx.closure_base_def_id(self.mir_def_id.to_def_id()); // If this is a closure or generator, then the late-bound regions from the enclosing // function are actually external regions to us. For example, here, 'a is not local @@ -426,7 +425,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { // fn foo<'a>() { // let c = || { let x: &'a u32 = ...; } // } - if self.mir_def_id != closure_base_def_id { + if self.mir_def_id.to_def_id() != closure_base_def_id { self.infcx.replace_late_bound_regions_with_nll_infer_vars(self.mir_def_id, &mut indices) } @@ -443,7 +442,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { ); // Converse of above, if this is a function then the late-bound regions declared on its // signature are local to the fn. - if self.mir_def_id == closure_base_def_id { + if self.mir_def_id.to_def_id() == closure_base_def_id { self.infcx .replace_late_bound_regions_with_nll_infer_vars(self.mir_def_id, &mut indices); } @@ -508,14 +507,14 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { /// see `DefiningTy` for details. fn defining_ty(&self) -> DefiningTy<'tcx> { let tcx = self.infcx.tcx; - let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id); + let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id.to_def_id()); match tcx.hir().body_owner_kind(self.mir_hir_id) { BodyOwnerKind::Closure | BodyOwnerKind::Fn => { - let defining_ty = if self.mir_def_id == closure_base_def_id { + let defining_ty = if self.mir_def_id.to_def_id() == closure_base_def_id { tcx.type_of(closure_base_def_id) } else { - let tables = tcx.typeck_tables_of(self.mir_def_id.expect_local()); + let tables = tcx.typeck_tables_of(self.mir_def_id); tables.node_type(self.mir_hir_id) }; @@ -540,11 +539,11 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { } BodyOwnerKind::Const | BodyOwnerKind::Static(..) => { - assert_eq!(closure_base_def_id, self.mir_def_id); + assert_eq!(self.mir_def_id.to_def_id(), closure_base_def_id); let identity_substs = InternalSubsts::identity_for_item(tcx, closure_base_def_id); let substs = self.infcx.replace_free_regions_with_nll_infer_vars(FR, &identity_substs); - DefiningTy::Const(self.mir_def_id, substs) + DefiningTy::Const(self.mir_def_id.to_def_id(), substs) } } } @@ -559,7 +558,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { defining_ty: DefiningTy<'tcx>, ) -> UniversalRegionIndices<'tcx> { let tcx = self.infcx.tcx; - let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id); + let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id.to_def_id()); let identity_substs = InternalSubsts::identity_for_item(tcx, closure_base_def_id); let fr_substs = match defining_ty { DefiningTy::Closure(_, ref substs) | DefiningTy::Generator(_, ref substs, _) => { @@ -593,7 +592,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { let tcx = self.infcx.tcx; match defining_ty { DefiningTy::Closure(def_id, substs) => { - assert_eq!(self.mir_def_id, def_id); + assert_eq!(self.mir_def_id.to_def_id(), def_id); let closure_sig = substs.as_closure().sig(); let inputs_and_output = closure_sig.inputs_and_output(); let closure_ty = tcx.closure_env_ty(def_id, substs).unwrap(); @@ -617,7 +616,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { } DefiningTy::Generator(def_id, substs, movability) => { - assert_eq!(self.mir_def_id, def_id); + assert_eq!(self.mir_def_id.to_def_id(), def_id); let resume_ty = substs.as_generator().resume_ty(); let output = substs.as_generator().return_ty(); let generator_ty = tcx.mk_generator(def_id, substs, movability); @@ -635,7 +634,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { DefiningTy::Const(def_id, _) => { // For a constant body, there are no inputs, and one // "output" (the type of the constant). - assert_eq!(self.mir_def_id, def_id); + assert_eq!(self.mir_def_id.to_def_id(), def_id); let ty = tcx.type_of(def_id); let ty = indices.fold_to_region_vids(tcx, &ty); ty::Binder::dummy(tcx.intern_type_list(&[ty])) @@ -656,7 +655,7 @@ trait InferCtxtExt<'tcx> { fn replace_bound_regions_with_nll_infer_vars( &self, origin: NLLRegionVariableOrigin, - all_outlive_scope: DefId, + all_outlive_scope: LocalDefId, value: &ty::Binder, indices: &mut UniversalRegionIndices<'tcx>, ) -> T @@ -665,7 +664,7 @@ trait InferCtxtExt<'tcx> { fn replace_late_bound_regions_with_nll_infer_vars( &self, - mir_def_id: DefId, + mir_def_id: LocalDefId, indices: &mut UniversalRegionIndices<'tcx>, ); } @@ -685,7 +684,7 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { fn replace_bound_regions_with_nll_infer_vars( &self, origin: NLLRegionVariableOrigin, - all_outlive_scope: DefId, + all_outlive_scope: LocalDefId, value: &ty::Binder, indices: &mut UniversalRegionIndices<'tcx>, ) -> T @@ -699,7 +698,7 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { let (value, _map) = self.tcx.replace_late_bound_regions(value, |br| { debug!("replace_bound_regions_with_nll_infer_vars: br={:?}", br); let liberated_region = self.tcx.mk_region(ty::ReFree(ty::FreeRegion { - scope: all_outlive_scope, + scope: all_outlive_scope.to_def_id(), bound_region: br, })); let region_vid = self.next_nll_region_var(origin); @@ -724,11 +723,11 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { /// inputs vector. fn replace_late_bound_regions_with_nll_infer_vars( &self, - mir_def_id: DefId, + mir_def_id: LocalDefId, indices: &mut UniversalRegionIndices<'tcx>, ) { debug!("replace_late_bound_regions_with_nll_infer_vars(mir_def_id={:?})", mir_def_id); - let closure_base_def_id = self.tcx.closure_base_def_id(mir_def_id); + let closure_base_def_id = self.tcx.closure_base_def_id(mir_def_id.to_def_id()); for_each_late_bound_region_defined_on(self.tcx, closure_base_def_id, |r| { debug!("replace_late_bound_regions_with_nll_infer_vars: r={:?}", r); if !indices.indices.contains_key(&r) { From 9308860a7b6dbeda34d2c116fd4109abe9969aca Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Sat, 27 Jun 2020 21:38:51 +0900 Subject: [PATCH 24/29] 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 25/29] 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 26/29] 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 27651490878c1ccd9714d7703efacf9744f5f84d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 24 Jun 2020 13:16:36 -0400 Subject: [PATCH 27/29] Serialize all foreign `SourceFile`s into proc-macro crate metadata Normally, we encode a `Span` that references a foreign `SourceFile` by encoding information about the foreign crate. When we decode this `Span`, we lookup the foreign crate in order to decode the `SourceFile`. However, this approach does not work for proc-macro crates. When we load a proc-macro crate, we do not deserialzie any of its dependencies (since a proc-macro crate can only export proc-macros). This means that we cannot serialize a reference to an upstream crate, since the associated metadata will not be available when we try to deserialize it. This commit modifies foreign span handling so that we treat all foreign `SourceFile`s as local `SourceFile`s when serializing a proc-macro. All `SourceFile`s will be stored into the metadata of a proc-macro crate, allowing us to cotinue to deserialize a proc-macro crate without needing to load any of its dependencies. Since the number of foreign `SourceFile`s that we load during a compilation session may be very large, we only serialize a `SourceFile` if we have also serialized a `Span` which requires it. --- src/librustc_metadata/rmeta/decoder.rs | 27 ++--- src/librustc_metadata/rmeta/encoder.rs | 102 +++++++++++++----- src/librustc_metadata/rmeta/mod.rs | 2 +- src/librustc_span/hygiene.rs | 3 +- src/librustc_span/source_map.rs | 43 +++++++- src/test/ui/hygiene/unpretty-debug.stdout | 4 +- .../ui/proc-macro/auxiliary/make-macro.rs | 10 ++ .../ui/proc-macro/auxiliary/meta-macro.rs | 12 +++ .../proc-macro/disappearing-resolution.stderr | 7 +- src/test/ui/proc-macro/meta-macro-hygiene.rs | 11 ++ .../ui/proc-macro/meta-macro-hygiene.stdout | 30 ++++++ src/test/ui/proc-macro/meta-macro.rs | 11 ++ src/test/ui/proc-macro/meta-macro.stdout | 1 + 13 files changed, 220 insertions(+), 43 deletions(-) create mode 100644 src/test/ui/proc-macro/auxiliary/make-macro.rs create mode 100644 src/test/ui/proc-macro/auxiliary/meta-macro.rs create mode 100644 src/test/ui/proc-macro/meta-macro-hygiene.rs create mode 100644 src/test/ui/proc-macro/meta-macro-hygiene.stdout create mode 100644 src/test/ui/proc-macro/meta-macro.rs create mode 100644 src/test/ui/proc-macro/meta-macro.stdout diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs index 25e57aa77acd0..ac1e1f1c0f831 100644 --- a/src/librustc_metadata/rmeta/decoder.rs +++ b/src/librustc_metadata/rmeta/decoder.rs @@ -450,19 +450,17 @@ impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { let imported_source_files = if tag == TAG_VALID_SPAN_LOCAL { self.cdata().imported_source_files(sess) } else { - // FIXME: We don't decode dependencies of proc-macros. - // Remove this once #69976 is merged + // When we encode a proc-macro crate, all `Span`s should be encoded + // with `TAG_VALID_SPAN_LOCAL` if self.cdata().root.is_proc_macro_crate() { - debug!( - "SpecializedDecoder::specialized_decode: skipping span for proc-macro crate {:?}", - self.cdata().cnum - ); // Decode `CrateNum` as u32 - using `CrateNum::decode` will ICE // since we don't have `cnum_map` populated. - // This advances the decoder position so that we can continue - // to read metadata. - let _ = u32::decode(self)?; - return Ok(DUMMY_SP); + let cnum = u32::decode(self)?; + panic!( + "Decoding of crate {:?} tried to access proc-macro dep {:?}", + self.cdata().root.name, + cnum + ); } // tag is TAG_VALID_SPAN_FOREIGN, checked by `debug_assert` above let cnum = CrateNum::decode(self)?; @@ -990,8 +988,13 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { DefKind::Macro(macro_kind(raw_macro)), self.local_def_id(def_index), ); - let ident = Ident::from_str(raw_macro.name()); - callback(Export { ident, res, vis: ty::Visibility::Public, span: DUMMY_SP }); + let ident = self.item_ident(def_index, sess); + callback(Export { + ident, + res, + vis: ty::Visibility::Public, + span: self.get_span(def_index, sess), + }); } } return; diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index cdc8b5e90a642..ab826e1555459 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -16,6 +16,7 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::itemlikevisit::{ItemLikeVisitor, ParItemLikeVisitor}; use rustc_hir::lang_items; use rustc_hir::{AnonConst, GenericParamKind}; +use rustc_index::bit_set::GrowableBitSet; use rustc_index::vec::Idx; use rustc_middle::hir::map::Map; use rustc_middle::middle::cstore::{EncodedMetadata, ForeignModule, LinkagePreference, NativeLib}; @@ -51,7 +52,20 @@ struct EncodeContext<'tcx> { interpret_allocs_inverse: Vec, // This is used to speed up Span encoding. - source_file_cache: Lrc, + // The `usize` is an index into the `MonotonicVec` + // that stores the `SourceFile` + source_file_cache: (Lrc, usize), + // The indices (into the `SourceMap`'s `MonotonicVec`) + // of all of the `SourceFiles` that we need to serialize. + // When we serialize a `Span`, we insert the index of its + // `SourceFile` into the `GrowableBitSet`. + // + // This needs to be a `GrowableBitSet` and not a + // regular `BitSet` because we may actually import new `SourceFiles` + // during metadata encoding, due to executing a query + // with a result containing a foreign `Span`. + required_source_files: Option>, + is_proc_macro: bool, } macro_rules! encoder_methods { @@ -154,18 +168,23 @@ impl<'tcx> SpecializedEncoder for EncodeContext<'tcx> { // The Span infrastructure should make sure that this invariant holds: debug_assert!(span.lo <= span.hi); - if !self.source_file_cache.contains(span.lo) { + if !self.source_file_cache.0.contains(span.lo) { let source_map = self.tcx.sess.source_map(); let source_file_index = source_map.lookup_source_file_idx(span.lo); - self.source_file_cache = source_map.files()[source_file_index].clone(); + self.source_file_cache = + (source_map.files()[source_file_index].clone(), source_file_index); } - if !self.source_file_cache.contains(span.hi) { + if !self.source_file_cache.0.contains(span.hi) { // Unfortunately, macro expansion still sometimes generates Spans // that malformed in this way. return TAG_INVALID_SPAN.encode(self); } + let source_files = self.required_source_files.as_mut().expect("Already encoded SourceMap!"); + // Record the fact that we need to encode the data for this `SourceFile` + source_files.insert(self.source_file_cache.1); + // There are two possible cases here: // 1. This span comes from a 'foreign' crate - e.g. some crate upstream of the // crate we are writing metadata for. When the metadata for *this* crate gets @@ -176,7 +195,13 @@ impl<'tcx> SpecializedEncoder for EncodeContext<'tcx> { // 2. This span comes from our own crate. No special hamdling is needed - we just // write `TAG_VALID_SPAN_LOCAL` to let the deserializer know that it should use // our own source map information. - let (tag, lo, hi) = if self.source_file_cache.is_imported() { + // + // If we're a proc-macro crate, we always treat this as a local `Span`. + // In `encode_source_map`, we serialize foreign `SourceFile`s into our metadata + // if we're a proc-macro crate. + // This allows us to avoid loading the dependencies of proc-macro crates: all of + // the information we need to decode `Span`s is stored in the proc-macro crate. + let (tag, lo, hi) = if self.source_file_cache.0.is_imported() && !self.is_proc_macro { // To simplify deserialization, we 'rebase' this span onto the crate it originally came from // (the crate that 'owns' the file it references. These rebased 'lo' and 'hi' values // are relative to the source map information for the 'foreign' crate whose CrateNum @@ -188,13 +213,13 @@ impl<'tcx> SpecializedEncoder for EncodeContext<'tcx> { // Span that can be used without any additional trouble. let external_start_pos = { // Introduce a new scope so that we drop the 'lock()' temporary - match &*self.source_file_cache.external_src.lock() { + match &*self.source_file_cache.0.external_src.lock() { ExternalSource::Foreign { original_start_pos, .. } => *original_start_pos, src => panic!("Unexpected external source {:?}", src), } }; - let lo = (span.lo - self.source_file_cache.start_pos) + external_start_pos; - let hi = (span.hi - self.source_file_cache.start_pos) + external_start_pos; + let lo = (span.lo - self.source_file_cache.0.start_pos) + external_start_pos; + let hi = (span.hi - self.source_file_cache.0.start_pos) + external_start_pos; (TAG_VALID_SPAN_FOREIGN, lo, hi) } else { @@ -212,7 +237,7 @@ impl<'tcx> SpecializedEncoder for EncodeContext<'tcx> { if tag == TAG_VALID_SPAN_FOREIGN { // This needs to be two lines to avoid holding the `self.source_file_cache` // while calling `cnum.encode(self)` - let cnum = self.source_file_cache.cnum; + let cnum = self.source_file_cache.0.cnum; cnum.encode(self)?; } Ok(()) @@ -386,17 +411,24 @@ impl<'tcx> EncodeContext<'tcx> { let all_source_files = source_map.files(); let (working_dir, _cwd_remapped) = self.tcx.sess.working_dir.clone(); + // By replacing the `Option` with `None`, we ensure that we can't + // accidentally serialize any more `Span`s after the source map encoding + // is done. + let required_source_files = self.required_source_files.take().unwrap(); let adapted = all_source_files .iter() - .filter(|source_file| { - // No need to re-export imported source_files, as any downstream - // crate will import them from their original source. - // FIXME(eddyb) the `Span` encoding should take that into account. - !source_file.is_imported() + .enumerate() + .filter(|(idx, source_file)| { + // Only serialize `SourceFile`s that were used + // during the encoding of a `Span` + required_source_files.contains(*idx) && + // Don't serialize imported `SourceFile`s, unless + // we're in a proc-macro crate. + (!source_file.is_imported() || self.is_proc_macro) }) - .map(|source_file| { - match source_file.name { + .map(|(_, source_file)| { + let mut adapted = match source_file.name { // This path of this SourceFile has been modified by // path-remapping, so we use it verbatim (and avoid // cloning the whole map in the process). @@ -419,15 +451,30 @@ impl<'tcx> EncodeContext<'tcx> { // expanded code, not from a file _ => source_file.clone(), + }; + + // We're serializing this `SourceFile` into our crate metadata, + // so mark it as coming from this crate. + // This also ensures that we don't try to deserialize the + // `CrateNum` for a proc-macro dependency - since proc macro + // dependencies aren't loaded when we deserialize a proc-macro, + // trying to remap the `CrateNum` would fail. + if self.is_proc_macro { + Lrc::make_mut(&mut adapted).cnum = LOCAL_CRATE; } + adapted }) .collect::>(); self.lazy(adapted.iter().map(|rc| &**rc)) } + fn is_proc_macro(&self) -> bool { + self.tcx.sess.crate_types().contains(&CrateType::ProcMacro) + } + fn encode_crate_root(&mut self) -> Lazy> { - let is_proc_macro = self.tcx.sess.crate_types().contains(&CrateType::ProcMacro); + let is_proc_macro = self.is_proc_macro(); let mut i = self.position(); @@ -458,11 +505,6 @@ impl<'tcx> EncodeContext<'tcx> { let foreign_modules = self.encode_foreign_modules(); - // Encode source_map - i = self.position(); - let source_map = self.encode_source_map(); - let source_map_bytes = self.position() - i; - // Encode DefPathTable i = self.position(); let def_path_table = self.encode_def_path_table(); @@ -514,12 +556,19 @@ impl<'tcx> EncodeContext<'tcx> { let proc_macro_data_bytes = self.position() - i; // Encode exported symbols info. This is prefetched in `encode_metadata` so we encode - // this last to give the prefetching as much time as possible to complete. + // this late to give the prefetching as much time as possible to complete. i = self.position(); let exported_symbols = self.tcx.exported_symbols(LOCAL_CRATE); let exported_symbols = self.encode_exported_symbols(&exported_symbols); let exported_symbols_bytes = self.position() - i; + // Encode source_map. This needs to be done last, + // since encoding `Span`s tells us which `SourceFiles` we actually + // need to encode. + i = self.position(); + let source_map = self.encode_source_map(); + let source_map_bytes = self.position() - i; + let attrs = tcx.hir().krate_attrs(); let has_default_lib_allocator = attr::contains_name(&attrs, sym::default_lib_allocator); @@ -1860,6 +1909,8 @@ fn encode_metadata_impl(tcx: TyCtxt<'_>) -> EncodedMetadata { // Will be filled with the root position after encoding everything. encoder.emit_raw_bytes(&[0, 0, 0, 0]); + let source_map_files = tcx.sess.source_map().files(); + let mut ecx = EncodeContext { opaque: encoder, tcx, @@ -1867,10 +1918,13 @@ fn encode_metadata_impl(tcx: TyCtxt<'_>) -> EncodedMetadata { lazy_state: LazyState::NoNode, type_shorthands: Default::default(), predicate_shorthands: Default::default(), - source_file_cache: tcx.sess.source_map().files()[0].clone(), + source_file_cache: (source_map_files[0].clone(), 0), interpret_allocs: Default::default(), interpret_allocs_inverse: Default::default(), + required_source_files: Some(GrowableBitSet::with_capacity(source_map_files.len())), + is_proc_macro: tcx.sess.crate_types().contains(&CrateType::ProcMacro), }; + drop(source_map_files); // Encode the rustc version string in a predictable location. rustc_version().encode(&mut ecx).unwrap(); diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index 0edea63f922d6..f0b35d0c62d5b 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -192,7 +192,6 @@ crate struct CrateRoot<'tcx> { diagnostic_items: Lazy<[(Symbol, DefIndex)]>, native_libraries: Lazy<[NativeLib]>, foreign_modules: Lazy<[ForeignModule]>, - source_map: Lazy<[rustc_span::SourceFile]>, def_path_table: Lazy, impls: Lazy<[TraitImpls]>, interpret_alloc_index: Lazy<[u32]>, @@ -203,6 +202,7 @@ crate struct CrateRoot<'tcx> { proc_macro_data: Option>, exported_symbols: Lazy!([(ExportedSymbol<'tcx>, SymbolExportLevel)]), + source_map: Lazy<[rustc_span::SourceFile]>, compiler_builtins: bool, needs_allocator: bool, diff --git a/src/librustc_span/hygiene.rs b/src/librustc_span/hygiene.rs index c0fb84e741f4a..c56c5bbb16f52 100644 --- a/src/librustc_span/hygiene.rs +++ b/src/librustc_span/hygiene.rs @@ -395,10 +395,11 @@ pub fn debug_hygiene_data(verbose: bool) -> String { data.expn_data.iter().enumerate().for_each(|(id, expn_info)| { let expn_info = expn_info.as_ref().expect("no expansion data for an expansion ID"); s.push_str(&format!( - "\n{}: parent: {:?}, call_site_ctxt: {:?}, kind: {:?}", + "\n{}: parent: {:?}, call_site_ctxt: {:?}, def_site_ctxt: {:?}, kind: {:?}", id, expn_info.parent, expn_info.call_site.ctxt(), + expn_info.def_site.ctxt(), expn_info.kind, )); }); diff --git a/src/librustc_span/source_map.rs b/src/librustc_span/source_map.rs index 4b5bce1db2628..e062c7766e7dc 100644 --- a/src/librustc_span/source_map.rs +++ b/src/librustc_span/source_map.rs @@ -40,6 +40,41 @@ pub fn original_sp(sp: Span, enclosing_sp: Span) -> Span { } } +pub mod monotonic { + use std::ops::{Deref, DerefMut}; + + /// A `MonotonicVec` is a `Vec` which can only be grown. + /// Once inserted, an element can never be removed or swapped, + /// guaranteeing that any indices into a `MonotonicVec` are stable + // This is declared in its own module to ensure that the private + // field is inaccessible + pub struct MonotonicVec(Vec); + impl MonotonicVec { + pub fn new(val: Vec) -> MonotonicVec { + MonotonicVec(val) + } + + pub fn push(&mut self, val: T) { + self.0.push(val); + } + } + + impl Default for MonotonicVec { + fn default() -> Self { + MonotonicVec::new(vec![]) + } + } + + impl Deref for MonotonicVec { + type Target = Vec; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl !DerefMut for MonotonicVec {} +} + #[derive(Clone, RustcEncodable, RustcDecodable, Debug, Copy, HashStable_Generic)] pub struct Spanned { pub node: T, @@ -125,7 +160,7 @@ impl StableSourceFileId { #[derive(Default)] pub(super) struct SourceMapFiles { - source_files: Vec>, + source_files: monotonic::MonotonicVec>, stable_id_to_source_file: FxHashMap>, } @@ -199,7 +234,9 @@ impl SourceMap { Ok(bytes) } - pub fn files(&self) -> MappedLockGuard<'_, Vec>> { + // By returning a `MonotonicVec`, we ensure that consumers cannot invalidate + // any existing indices pointing into `files`. + pub fn files(&self) -> MappedLockGuard<'_, monotonic::MonotonicVec>> { LockGuard::map(self.files.borrow(), |files| &mut files.source_files) } @@ -912,6 +949,8 @@ impl SourceMap { } // Returns the index of the `SourceFile` (in `self.files`) that contains `pos`. + // This index is guaranteed to be valid for the lifetime of this `SourceMap`, + // since `source_files` is a `MonotonicVec` pub fn lookup_source_file_idx(&self, pos: BytePos) -> usize { self.files .borrow() diff --git a/src/test/ui/hygiene/unpretty-debug.stdout b/src/test/ui/hygiene/unpretty-debug.stdout index acd852103cae3..81164030d8eef 100644 --- a/src/test/ui/hygiene/unpretty-debug.stdout +++ b/src/test/ui/hygiene/unpretty-debug.stdout @@ -16,8 +16,8 @@ fn y /* 0#0 */() { } /* Expansions: -0: parent: ExpnId(0), call_site_ctxt: #0, kind: Root -1: parent: ExpnId(0), call_site_ctxt: #0, kind: Macro(Bang, "foo") +0: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Root +1: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "foo") SyntaxContexts: #0: parent: #0, outer_mark: (ExpnId(0), Opaque) diff --git a/src/test/ui/proc-macro/auxiliary/make-macro.rs b/src/test/ui/proc-macro/auxiliary/make-macro.rs new file mode 100644 index 0000000000000..155d54ea0bfb0 --- /dev/null +++ b/src/test/ui/proc-macro/auxiliary/make-macro.rs @@ -0,0 +1,10 @@ +#[macro_export] +macro_rules! make_it { + ($name:ident) => { + #[proc_macro] + pub fn $name(input: TokenStream) -> TokenStream { + println!("Def site: {:?}", Span::def_site()); + input + } + }; +} diff --git a/src/test/ui/proc-macro/auxiliary/meta-macro.rs b/src/test/ui/proc-macro/auxiliary/meta-macro.rs new file mode 100644 index 0000000000000..5265c6533b479 --- /dev/null +++ b/src/test/ui/proc-macro/auxiliary/meta-macro.rs @@ -0,0 +1,12 @@ +// force-host +// no-prefer-dynamic +// edition:2018 + +#![feature(proc_macro_def_site)] +#![crate_type = "proc-macro"] + +extern crate proc_macro; +extern crate make_macro; +use proc_macro::{TokenStream, Span}; + +make_macro::make_it!(print_def_site); diff --git a/src/test/ui/proc-macro/disappearing-resolution.stderr b/src/test/ui/proc-macro/disappearing-resolution.stderr index ff7ddcde6e0c4..5b969549a117c 100644 --- a/src/test/ui/proc-macro/disappearing-resolution.stderr +++ b/src/test/ui/proc-macro/disappearing-resolution.stderr @@ -10,11 +10,16 @@ error[E0603]: derive macro import `Empty` is private LL | use m::Empty; | ^^^^^ private derive macro import | -note: the derive macro import `Empty` is defined here +note: the derive macro import `Empty` is defined here... --> $DIR/disappearing-resolution.rs:9:9 | LL | use test_macros::Empty; | ^^^^^^^^^^^^^^^^^^ +note: ...and refers to the derive macro `Empty` which is defined here + --> $DIR/auxiliary/test-macros.rs:25:1 + | +LL | pub fn empty_derive(_: TokenStream) -> TokenStream { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ consider importing it directly error: aborting due to 2 previous errors diff --git a/src/test/ui/proc-macro/meta-macro-hygiene.rs b/src/test/ui/proc-macro/meta-macro-hygiene.rs new file mode 100644 index 0000000000000..97401448911a3 --- /dev/null +++ b/src/test/ui/proc-macro/meta-macro-hygiene.rs @@ -0,0 +1,11 @@ +// aux-build:make-macro.rs +// aux-build:meta-macro.rs +// edition:2018 +// compile-flags: -Z span-debug -Z unpretty=expanded,hygiene +// check-pass + +extern crate meta_macro; + +fn main() { + meta_macro::print_def_site!(); +} diff --git a/src/test/ui/proc-macro/meta-macro-hygiene.stdout b/src/test/ui/proc-macro/meta-macro-hygiene.stdout new file mode 100644 index 0000000000000..b3a20763dc5e6 --- /dev/null +++ b/src/test/ui/proc-macro/meta-macro-hygiene.stdout @@ -0,0 +1,30 @@ +Def site: $DIR/auxiliary/make-macro.rs:5:9: 8:10 (#3) +#![feature /* 280#0 */(prelude_import)] +#[prelude_import /* 527#1 */] +use std /* 687#1 */::prelude /* 526#1 */::v1 /* 783#1 */::*; +#[macro_use /* 404#1 */] +extern crate std /* 687#1 */; +// aux-build:make-macro.rs +// aux-build:meta-macro.rs +// edition:2018 +// compile-flags: -Z span-debug -Z unpretty=expanded,hygiene +// check-pass + +extern crate meta_macro /* 834#0 */; + +fn main /* 406#0 */() { } + +/* +Expansions: +0: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Root +1: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: AstPass(StdImports) +2: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "meta_macro::print_def_site") + +SyntaxContexts: +#0: parent: #0, outer_mark: (ExpnId(0), Opaque) +#1: parent: #0, outer_mark: (ExpnId(1), Opaque) +#2: parent: #0, outer_mark: (ExpnId(1), Transparent) +#3: parent: #0, outer_mark: (ExpnId(2), Opaque) +#4: parent: #0, outer_mark: (ExpnId(2), Transparent) +#5: parent: #0, outer_mark: (ExpnId(2), SemiTransparent) +*/ diff --git a/src/test/ui/proc-macro/meta-macro.rs b/src/test/ui/proc-macro/meta-macro.rs new file mode 100644 index 0000000000000..dbfde9e113f37 --- /dev/null +++ b/src/test/ui/proc-macro/meta-macro.rs @@ -0,0 +1,11 @@ +// aux-build:make-macro.rs +// aux-build:meta-macro.rs +// edition:2018 +// compile-flags: -Z span-debug +// run-pass + +extern crate meta_macro; + +fn main() { + meta_macro::print_def_site!(); +} diff --git a/src/test/ui/proc-macro/meta-macro.stdout b/src/test/ui/proc-macro/meta-macro.stdout new file mode 100644 index 0000000000000..00439bc292081 --- /dev/null +++ b/src/test/ui/proc-macro/meta-macro.stdout @@ -0,0 +1 @@ +Def site: $DIR/auxiliary/make-macro.rs:5:9: 8:10 (#3) From 0e0584f290b5194e790a5d324cbab55a975ce714 Mon Sep 17 00:00:00 2001 From: Rob Young Date: Sat, 27 Jun 2020 19:32:19 +0100 Subject: [PATCH 28/29] Add links to fs::DirEntry::metadata `fs::DirEntry::metadata` doesn't traverse symlinks. It is not immediately clear what to do if you do want to traverse symlinks. This change adds links to the two other `metadata` functions that will follow symlinks. --- src/libstd/fs.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index f4c164a324e32..2a081fa65ca0e 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1429,7 +1429,10 @@ impl DirEntry { /// Returns the metadata for the file that this entry points at. /// /// This function will not traverse symlinks if this entry points at a - /// symlink. + /// symlink. To traverse symlinks use [`fs::metadata`] or [`fs::File::metadata`]. + /// + /// [`fs::metadata`]: fn.metadata.html + /// [`fs::File::metadata`]: struct.File.html#method.metadata /// /// # Platform-specific behavior /// From 8e8c54aa3a8d92d8443ec4596754d14b2d196899 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 27 Jun 2020 22:59:47 +0200 Subject: [PATCH 29/29] 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)