From 6bbe162c4d83f98e77e6646dfa95e0f9957dd257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Mon, 28 Oct 2019 00:00:00 +0000 Subject: [PATCH 01/29] Optimize long-linker-command-line test Replace O(n^3) text matching with inexpensive hash set lookups. On my machine this reduces the total runtime of complete run-make-fulldeps suite from roughly 75 seconds to 45 seconds. --- .../long-linker-command-lines/foo.rs | 73 +++++++++++-------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/src/test/run-make-fulldeps/long-linker-command-lines/foo.rs b/src/test/run-make-fulldeps/long-linker-command-lines/foo.rs index ac42141365e9d..96fb16b1fcc8f 100644 --- a/src/test/run-make-fulldeps/long-linker-command-lines/foo.rs +++ b/src/test/run-make-fulldeps/long-linker-command-lines/foo.rs @@ -7,12 +7,43 @@ // Eventually we should see an argument that looks like `@` as we switch from // passing literal arguments to passing everything in the file. +use std::collections::HashSet; use std::env; use std::fs::{self, File}; -use std::io::{BufWriter, Write, Read}; -use std::path::PathBuf; +use std::io::{BufWriter, Write}; +use std::path::{Path, PathBuf}; use std::process::Command; +fn write_test_case(file: &Path, n: usize) -> HashSet { + let mut libs = HashSet::new(); + let mut f = BufWriter::new(File::create(&file).unwrap()); + let mut prefix = String::new(); + for _ in 0..n { + prefix.push_str("foo"); + } + for i in 0..n { + writeln!(f, "#[link(name = \"S{}{}S\")]", prefix, i).unwrap(); + libs.insert(format!("{}{}", prefix, i)); + } + writeln!(f, "extern {{}}\nfn main() {{}}").unwrap(); + f.into_inner().unwrap(); + + libs +} + +fn read_linker_args(path: &Path) -> String { + let contents = fs::read(path).unwrap(); + if cfg!(target_env = "msvc") { + let mut i = contents.chunks(2).map(|c| { + c[0] as u16 | ((c[1] as u16) << 8) + }); + assert_eq!(i.next(), Some(0xfeff), "Expected UTF-16 BOM"); + String::from_utf16(&i.collect::>()).unwrap() + } else { + String::from_utf8(contents).unwrap() + } +} + fn main() { let tmpdir = PathBuf::from(env::var_os("TMPDIR").unwrap()); let ok = tmpdir.join("ok"); @@ -29,16 +60,7 @@ fn main() { for i in (1..).map(|i| i * 100) { println!("attempt: {}", i); let file = tmpdir.join("bar.rs"); - let mut f = BufWriter::new(File::create(&file).unwrap()); - let mut lib_name = String::new(); - for _ in 0..i { - lib_name.push_str("foo"); - } - for j in 0..i { - writeln!(f, "#[link(name = \"{}{}\")]", lib_name, j).unwrap(); - } - writeln!(f, "extern {{}}\nfn main() {{}}").unwrap(); - f.into_inner().unwrap(); + let mut expected_libs = write_test_case(&file, i); drop(fs::remove_file(&ok)); let output = Command::new(&rustc) @@ -67,25 +89,18 @@ fn main() { continue } - let mut contents = Vec::new(); - File::open(&ok).unwrap().read_to_end(&mut contents).unwrap(); - - for j in 0..i { - let exp = format!("{}{}", lib_name, j); - let exp = if cfg!(target_env = "msvc") { - let mut out = Vec::with_capacity(exp.len() * 2); - for c in exp.encode_utf16() { - // encode in little endian - out.push(c as u8); - out.push((c >> 8) as u8); - } - out - } else { - exp.into_bytes() - }; - assert!(contents.windows(exp.len()).any(|w| w == &exp[..])); + let linker_args = read_linker_args(&ok); + for mut arg in linker_args.split('S') { + expected_libs.remove(arg); } + assert!( + expected_libs.is_empty(), + "expected but missing libraries: {:#?}\nlinker arguments: \n{}", + expected_libs, + linker_args, + ); + break } } From df4e12d88947db6ff832bb7caae44927af687eb7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Nov 2019 11:56:06 +0100 Subject: [PATCH 02/29] uninit/zeroed lint: warn against NULL vtables --- src/librustc_lint/builtin.rs | 2 ++ src/librustc_lint/lib.rs | 1 + src/test/ui/lint/uninitialized-zeroed.rs | 3 ++ src/test/ui/lint/uninitialized-zeroed.stderr | 36 ++++++++++++++++---- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index e3c3966c2f5e0..e1c57d8afffb8 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1949,6 +1949,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { Adt(..) if ty.is_box() => Some((format!("`Box` must be non-null"), None)), FnPtr(..) => Some((format!("Function pointers must be non-null"), None)), Never => Some((format!("The never type (`!`) has no valid value"), None)), + RawPtr(tm) if matches!(tm.ty.kind, Dynamic(..)) => // raw ptr to dyn Trait + Some((format!("The vtable of a wide raw pointer must be non-null"), None)), // Primitive types with other constraints. Bool if init == InitKind::Uninit => Some((format!("Booleans must be `true` or `false`"), None)), diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index b1beef04c5929..a47980c5ead30 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -15,6 +15,7 @@ #![feature(box_patterns)] #![feature(box_syntax)] #![feature(nll)] +#![feature(matches_macro)] #![recursion_limit="256"] diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs index 5cf62b8691239..ccc4e77bc97d6 100644 --- a/src/test/ui/lint/uninitialized-zeroed.rs +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -67,6 +67,9 @@ fn main() { let _val: NonNull = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: NonNull = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: *const dyn Send = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: *const dyn Send = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + // Things that can be zero, but not uninit. let _val: bool = mem::zeroed(); let _val: bool = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr index a36a32a39a11b..85b1e0aaff00e 100644 --- a/src/test/ui/lint/uninitialized-zeroed.stderr +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -307,8 +307,30 @@ LL | let _val: NonNull = mem::uninitialized(); | = note: std::ptr::NonNull must be non-null +error: the type `*const dyn std::marker::Send` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:70:37 + | +LL | let _val: *const dyn Send = mem::zeroed(); + | ^^^^^^^^^^^^^ + | | + | this code causes undefined behavior when executed + | help: use `MaybeUninit` instead + | + = note: The vtable of a wide raw pointer must be non-null + +error: the type `*const dyn std::marker::Send` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:71:37 + | +LL | let _val: *const dyn Send = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | | + | this code causes undefined behavior when executed + | help: use `MaybeUninit` instead + | + = note: The vtable of a wide raw pointer must be non-null + error: the type `bool` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:72:26 + --> $DIR/uninitialized-zeroed.rs:75:26 | LL | let _val: bool = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ @@ -319,7 +341,7 @@ LL | let _val: bool = mem::uninitialized(); = note: Booleans must be `true` or `false` error: the type `Wrap` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:75:32 + --> $DIR/uninitialized-zeroed.rs:78:32 | LL | let _val: Wrap = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ @@ -334,7 +356,7 @@ LL | struct Wrap { wrapped: T } | ^^^^^^^^^^ error: the type `NonBig` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:78:28 + --> $DIR/uninitialized-zeroed.rs:81:28 | LL | let _val: NonBig = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ @@ -345,7 +367,7 @@ LL | let _val: NonBig = mem::uninitialized(); = note: NonBig must be initialized inside its custom valid range error: the type `&'static i32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:81:34 + --> $DIR/uninitialized-zeroed.rs:84:34 | LL | let _val: &'static i32 = mem::transmute(0usize); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -356,7 +378,7 @@ LL | let _val: &'static i32 = mem::transmute(0usize); = note: References must be non-null error: the type `&'static [i32]` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:82:36 + --> $DIR/uninitialized-zeroed.rs:85:36 | LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -367,7 +389,7 @@ LL | let _val: &'static [i32] = mem::transmute((0usize, 0usize)); = note: References must be non-null error: the type `std::num::NonZeroU32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:83:32 + --> $DIR/uninitialized-zeroed.rs:86:32 | LL | let _val: NonZeroU32 = mem::transmute(0); | ^^^^^^^^^^^^^^^^^ @@ -377,5 +399,5 @@ LL | let _val: NonZeroU32 = mem::transmute(0); | = note: std::num::NonZeroU32 must be non-null -error: aborting due to 30 previous errors +error: aborting due to 32 previous errors From 7ff57edb93625857b1ac289160550859e78ef6fb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Nov 2019 16:12:33 +0100 Subject: [PATCH 03/29] also identiy MaybeUninit::uninit().assume_init() as dangerous --- src/librustc/lint/context.rs | 3 ++ src/librustc_lint/builtin.rs | 23 +++++++++++-- src/libsyntax_pos/symbol.rs | 4 +++ src/test/ui/lint/uninitialized-zeroed.rs | 6 ++++ src/test/ui/lint/uninitialized-zeroed.stderr | 35 +++++++++++++++++++- 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 1cb53d754dcd3..99a45c4e76104 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -699,6 +699,9 @@ impl<'a, 'tcx> LateContext<'a, 'tcx> { /// Check if a `DefId`'s path matches the given absolute type path usage. /// + /// Anonymous scopes such as `extern` imports are matched with `kw::Invalid`; + /// inherent `impl` blocks are matched with the name of the type. + /// /// # Examples /// /// ```rust,ignore (no context or def id available) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index e1c57d8afffb8..7f7db46466f38 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1911,8 +1911,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { // `Invalid` represents the empty string and matches that. const TRANSMUTE_PATH: &[Symbol] = &[sym::core, sym::intrinsics, kw::Invalid, sym::transmute]; + const MU_ZEROED_PATH: &[Symbol] = + &[sym::core, sym::mem, sym::maybe_uninit, sym::MaybeUninit, sym::zeroed]; + const MU_UNINIT_PATH: &[Symbol] = + &[sym::core, sym::mem, sym::maybe_uninit, sym::MaybeUninit, sym::uninit]; if let hir::ExprKind::Call(ref path_expr, ref args) = expr.kind { + // Find calls to `mem::{uninitialized,zeroed}` methods. if let hir::ExprKind::Path(ref qpath) = path_expr.kind { let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?; @@ -1927,8 +1932,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { return Some(InitKind::Zeroed); } } - // FIXME: Also detect `MaybeUninit::zeroed().assume_init()` and - // `MaybeUninit::uninit().assume_init()`. + } + } else if let hir::ExprKind::MethodCall(ref path, _, ref args) = expr.kind { + // Find problematic calls to `MaybeUninit::assume_init`. + if path.ident.name == sym::assume_init { + // This is a call to *some* method named `assume_init`. + // See if the `self` parameter is one of the dangerous constructors. + if let hir::ExprKind::Call(ref path_expr, _) = args[0].kind { + if let hir::ExprKind::Path(ref qpath) = path_expr.kind { + let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?; + if cx.match_def_path(def_id, MU_ZEROED_PATH) { + return Some(InitKind::Zeroed); + } else if cx.match_def_path(def_id, MU_UNINIT_PATH) { + return Some(InitKind::Uninit); + } + } + } } } diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 57131ffe18cb3..e3165a457b42b 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -148,6 +148,7 @@ symbols! { associated_type_bounds, associated_type_defaults, associated_types, + assume_init, async_await, async_closure, attr, @@ -417,6 +418,8 @@ symbols! { match_beginning_vert, match_default_bindings, may_dangle, + maybe_uninit, + MaybeUninit, mem, member_constraints, message, @@ -709,6 +712,7 @@ symbols! { underscore_imports, underscore_lifetimes, uniform_paths, + uninit, uninitialized, universal_impl_trait, unmarked_api, diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs index ccc4e77bc97d6..473be434a7524 100644 --- a/src/test/ui/lint/uninitialized-zeroed.rs +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -85,10 +85,16 @@ fn main() { let _val: &'static [i32] = mem::transmute((0usize, 0usize)); //~ ERROR: does not permit zero-initialization let _val: NonZeroU32 = mem::transmute(0); //~ ERROR: does not permit zero-initialization + // `MaybeUninit` cases + let _val: NonNull = MaybeUninit::zeroed().assume_init(); //~ ERROR: does not permit zero-initialization + let _val: NonNull = MaybeUninit::uninit().assume_init(); //~ ERROR: does not permit being left uninitialized + let _val: bool = MaybeUninit::uninit().assume_init(); //~ ERROR: does not permit being left uninitialized + // Some more types that should work just fine. let _val: Option<&'static i32> = mem::zeroed(); let _val: Option = mem::zeroed(); let _val: MaybeUninit<&'static i32> = mem::zeroed(); let _val: i32 = mem::zeroed(); + let _val: bool = MaybeUninit::zeroed().assume_init(); } } diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr index 85b1e0aaff00e..e12b1897ade1b 100644 --- a/src/test/ui/lint/uninitialized-zeroed.stderr +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -399,5 +399,38 @@ LL | let _val: NonZeroU32 = mem::transmute(0); | = note: std::num::NonZeroU32 must be non-null -error: aborting due to 32 previous errors +error: the type `std::ptr::NonNull` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:89:34 + | +LL | let _val: NonNull = MaybeUninit::zeroed().assume_init(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | this code causes undefined behavior when executed + | help: use `MaybeUninit` instead + | + = note: std::ptr::NonNull must be non-null + +error: the type `std::ptr::NonNull` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:90:34 + | +LL | let _val: NonNull = MaybeUninit::uninit().assume_init(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | this code causes undefined behavior when executed + | help: use `MaybeUninit` instead + | + = note: std::ptr::NonNull must be non-null + +error: the type `bool` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:91:26 + | +LL | let _val: bool = MaybeUninit::uninit().assume_init(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | this code causes undefined behavior when executed + | help: use `MaybeUninit` instead + | + = note: Booleans must be `true` or `false` + +error: aborting due to 35 previous errors From 65153710e1cc46a33b65f90cd92732bc9c77cee5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Nov 2019 09:50:59 +0100 Subject: [PATCH 04/29] QPath docs: mention how to resolve them --- src/librustc/hir/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 0edc41e6b4881..d28624907d717 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1698,6 +1698,10 @@ pub enum ExprKind { } /// Represents an optionally `Self`-qualified value/type path or associated extension. +/// +/// To resolve the path to a `DefId`, call [`qpath_res`]. +/// +/// [`qpath_res`]: ty/struct.TypeckTables.html#method.qpath_res #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum QPath { /// Path to a definition, optionally "fully-qualified" with a `Self` From bb37d0078750b760f013bfa706fe19d4d823b8df Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Nov 2019 10:11:58 +0100 Subject: [PATCH 05/29] more robust method checking through DefId and diagnostic_item --- src/libcore/mem/maybe_uninit.rs | 1 + src/librustc_lint/builtin.rs | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index 792ce9dfad419..339a94c218005 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -440,6 +440,7 @@ impl MaybeUninit { /// ``` #[stable(feature = "maybe_uninit", since = "1.36.0")] #[inline(always)] + #[cfg_attr(all(not(bootstrap)), rustc_diagnostic_item = "assume_init")] pub unsafe fn assume_init(self) -> T { intrinsics::panic_if_uninhabited::(); ManuallyDrop::into_inner(self.value) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 7f7db46466f38..e38ecd882830d 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1933,9 +1933,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { } } } - } else if let hir::ExprKind::MethodCall(ref path, _, ref args) = expr.kind { + } else if let hir::ExprKind::MethodCall(_, _, ref args) = expr.kind { // Find problematic calls to `MaybeUninit::assume_init`. - if path.ident.name == sym::assume_init { + let def_id = cx.tables.type_dependent_def_id(expr.hir_id)?; + if cx.tcx.is_diagnostic_item(sym::assume_init, def_id) { // This is a call to *some* method named `assume_init`. // See if the `self` parameter is one of the dangerous constructors. if let hir::ExprKind::Call(ref path_expr, _) = args[0].kind { From 151e9890f4cf8ea714dd6a4760589b322dab567b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Nov 2019 10:16:16 +0100 Subject: [PATCH 06/29] also explain how to resolve MethodCall --- src/librustc/hir/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index d28624907d717..cbbf808bece06 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1616,6 +1616,11 @@ pub enum ExprKind { /// and the remaining elements are the rest of the arguments. /// Thus, `x.foo::(a, b, c, d)` is represented as /// `ExprKind::MethodCall(PathSegment { foo, [Bar, Baz] }, [x, a, b, c, d])`. + /// + /// To resolve the called method to a `DefId`, call [`type_dependent_def_id`] with + /// the `hir_id` of the `MethodCall` node itself. + /// + /// [`qpath_res`]: ../ty/struct.TypeckTables.html#method.type_dependent_def_id MethodCall(P, Span, HirVec), /// A tuple (e.g., `(a, b, c, d)`). Tup(HirVec), @@ -1701,7 +1706,7 @@ pub enum ExprKind { /// /// To resolve the path to a `DefId`, call [`qpath_res`]. /// -/// [`qpath_res`]: ty/struct.TypeckTables.html#method.qpath_res +/// [`qpath_res`]: ../ty/struct.TypeckTables.html#method.qpath_res #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum QPath { /// Path to a definition, optionally "fully-qualified" with a `Self` From d22a65995ad10760ce664cb40e98139787316c99 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 4 Nov 2019 16:53:05 +0300 Subject: [PATCH 07/29] Do not require extra LLVM backends for `x.py test` to pass --- src/test/codegen/abi-efiapi.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/codegen/abi-efiapi.rs b/src/test/codegen/abi-efiapi.rs index 72adb95e96af9..8aeee5859d0ad 100644 --- a/src/test/codegen/abi-efiapi.rs +++ b/src/test/codegen/abi-efiapi.rs @@ -1,14 +1,12 @@ // Checks if the correct annotation for the efiapi ABI is passed to llvm. -// revisions:x86_64 i686 aarch64 arm riscv +// revisions:x86_64 i686 arm // min-llvm-version 9.0 //[x86_64] compile-flags: --target x86_64-unknown-uefi //[i686] compile-flags: --target i686-unknown-linux-musl -//[aarch64] compile-flags: --target aarch64-unknown-none //[arm] compile-flags: --target armv7r-none-eabi -//[riscv] compile-flags: --target riscv64gc-unknown-none-elf // compile-flags: -C no-prepopulate-passes #![crate_type = "lib"] @@ -24,8 +22,6 @@ trait Copy { } //x86_64: define win64cc void @has_efiapi //i686: define void @has_efiapi -//aarch64: define void @has_efiapi //arm: define void @has_efiapi -//riscv: define void @has_efiapi #[no_mangle] pub extern "efiapi" fn has_efiapi() {} From 82dc3aa5fb6642fe0450305015ee68e0c2d1d492 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 09:32:47 +0100 Subject: [PATCH 08/29] link from raw slice creation methods to safety requirements --- src/libcore/ptr/mod.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 1355ce1aa43b7..649244a468305 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -221,10 +221,15 @@ pub(crate) struct FatPtr { pub(crate) len: usize, } -/// Forms a slice from a pointer and a length. +/// Forms a raw slice from a pointer and a length. /// /// The `len` argument is the number of **elements**, not the number of bytes. /// +/// This function is safe, but actually using the return value is unsafe. +/// See the documentation of [`from_raw_parts`] for slice safety requirements. +/// +/// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html +/// /// # Examples /// /// ```rust @@ -243,12 +248,16 @@ pub fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { unsafe { Repr { raw: FatPtr { data, len } }.rust } } -/// Performs the same functionality as [`from_raw_parts`], except that a -/// mutable slice is returned. +/// Performs the same functionality as [`slice_from_raw_parts`], except that a +/// raw mutable slice is returned. /// -/// See the documentation of [`from_raw_parts`] for more details. +/// See the documentation of [`slice_from_raw_parts`] for more details. /// -/// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html +/// This function is safe, but actually using the return value is unsafe. +/// See the documentation of [`from_raw_parts_mut`] for slice safety requirements. +/// +/// [`slice_from_raw_parts`]: fn.slice_from_raw_parts.html +/// [`from_raw_parts_mut`]: ../../std/slice/fn.from_raw_parts_mut.html #[inline] #[unstable(feature = "slice_from_raw_parts", reason = "recently added", issue = "36925")] pub fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { From 1a254e4f434e0cbb9ffcb24971416cb574df4751 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 09:55:33 +0100 Subject: [PATCH 09/29] expand slice from_raw_part docs --- src/libcore/ptr/mod.rs | 4 +++ src/libcore/slice/mod.rs | 60 ++++++++++++++++++++++++++++------------ 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 649244a468305..407f8a6218e4f 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -18,6 +18,10 @@ //! * A [null] pointer is *never* valid, not even for accesses of [size zero][zst]. //! * All pointers (except for the null pointer) are valid for all operations of //! [size zero][zst]. +//! * For a pointer to be valid, it is necessary (but not always sufficient) that the pointer +//! be *dereferencable*: the memory range of the given size starting at the pointer must all be +//! within the bounds of a single allocated object. Note that in Rust, +//! every (stack-allocated) variable is considered a separate allocated object. //! * All accesses performed by functions in this module are *non-atomic* in the sense //! of [atomic operations] used to synchronize between threads. This means it is //! undefined behavior to perform two concurrent accesses to the same location from different diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index cdada1252d2bf..a7b759f523103 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -5272,18 +5272,24 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { /// /// # Safety /// -/// This function is unsafe as there is no guarantee that the given pointer is -/// valid for `len` elements, nor whether the lifetime inferred is a suitable -/// lifetime for the returned slice. +/// Behavior is undefined if any of the following conditions are violated: /// -/// `data` must be non-null and aligned, even for zero-length slices. One -/// reason for this is that enum layout optimizations may rely on references -/// (including slices of any length) being aligned and non-null to distinguish -/// them from other data. You can obtain a pointer that is usable as `data` -/// for zero-length slices using [`NonNull::dangling()`]. +/// * `data` must be [valid] for reads for `len * mem::size_of::()` many bytes, +/// and it must be properly aligned. This means in particular: /// -/// The total size of the slice must be no larger than `isize::MAX` **bytes** -/// in memory. See the safety documentation of [`pointer::offset`]. +/// * The entire memory range of this slice must be contained within a single allocated object! +/// Slices can never span across multiple allocated objects. +/// * `data` must be non-null and aligned even for zero-length slices. One +/// reason for this is that enum layout optimizations may rely on references +/// (including slices of any length) being aligned and non-null to distinguish +/// them from other data. You can obtain a pointer that is usable as `data` +/// for zero-length slices using [`NonNull::dangling()`]. +/// +/// * The memory referenced by the returned slice must not be mutated for the duration +/// of lifetime `'a`, except inside an `UnsafeCell`. +/// +/// * The total size `len * mem::size_of::()` of the slice must be no larger than `isize::MAX`. +/// See the safety documentation of [`pointer::offset`]. /// /// # Caveat /// @@ -5305,6 +5311,7 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { /// assert_eq!(slice[0], 42); /// ``` /// +/// [valid]: ../ptr/index.html#safety /// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling /// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset #[inline] @@ -5312,28 +5319,45 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] { debug_assert!(is_aligned_and_not_null(data), "attempt to create unaligned or null slice"); debug_assert!(mem::size_of::().saturating_mul(len) <= isize::MAX as usize, - "attempt to create slice covering half the address space"); + "attempt to create slice covering at least half the address space"); &*ptr::slice_from_raw_parts(data, len) } /// Performs the same functionality as [`from_raw_parts`], except that a /// mutable slice is returned. /// -/// This function is unsafe for the same reasons as [`from_raw_parts`], as well -/// as not being able to provide a non-aliasing guarantee of the returned -/// mutable slice. `data` must be non-null and aligned even for zero-length -/// slices as with [`from_raw_parts`]. The total size of the slice must be no -/// larger than `isize::MAX` **bytes** in memory. +/// # Safety +/// +/// Behavior is undefined if any of the following conditions are violated: +/// +/// * `data` must be [valid] for writes for `len * mem::size_of::()` many bytes, +/// and it must be properly aligned. This means in particular: /// -/// See the documentation of [`from_raw_parts`] for more details. +/// * The entire memory range of this slice must be contained within a single allocated object! +/// Slices can never span across multiple allocated objects. +/// * `data` must be non-null and aligned even for zero-length slices. One +/// reason for this is that enum layout optimizations may rely on references +/// (including slices of any length) being aligned and non-null to distinguish +/// them from other data. You can obtain a pointer that is usable as `data` +/// for zero-length slices using [`NonNull::dangling()`]. /// +/// * The memory referenced by the returned slice must not be accessed through any other pointer +/// (not derived from the return value) for the duration of lifetime `'a`. +/// Both read and write accesses are forbidden. +/// +/// * The total size `len * mem::size_of::()` of the slice must be no larger than `isize::MAX`. +/// See the safety documentation of [`pointer::offset`]. +/// +/// [valid]: ../ptr/index.html#safety +/// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling +/// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset /// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] { debug_assert!(is_aligned_and_not_null(data), "attempt to create unaligned or null slice"); debug_assert!(mem::size_of::().saturating_mul(len) <= isize::MAX as usize, - "attempt to create slice covering half the address space"); + "attempt to create slice covering at least half the address space"); &mut *ptr::slice_from_raw_parts_mut(data, len) } From 6a1f303b98fe77775f3f201bef792a98e31e79e8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 09:57:52 +0100 Subject: [PATCH 10/29] also edit String::from_raw_parts while we are at it --- src/liballoc/string.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index d9927c642b2d8..62e58b6facd01 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -687,7 +687,7 @@ impl String { /// checked: /// /// * The memory at `ptr` needs to have been previously allocated by the - /// same allocator the standard library uses. + /// same allocator the standard library uses, with a required alignment of exactly 1. /// * `length` needs to be less than or equal to `capacity`. /// * `capacity` needs to be the correct value. /// From 002c1c74d99295e49e30a02fb98f25383f1576f8 Mon Sep 17 00:00:00 2001 From: Pyry Kontio Date: Tue, 5 Nov 2019 19:16:09 +0900 Subject: [PATCH 11/29] Improve std::thread::Result documentation --- src/libstd/thread/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 0ffa6ace2e4d2..2af0a4804ef42 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -1271,6 +1271,18 @@ impl fmt::Debug for Thread { /// /// Indicates the manner in which a thread exited. /// +/// The value contained in the `Result::Err` variant +/// is the value the thread panicked with; +/// that is, the parameter the `panic!` macro was called with. +/// Unlike with normal errors, this value doesn't implement +/// the `std::error::Error` trait. +/// +/// Thus, a sensible way to handle a thread panic is to either +/// `unwrap` the `Result`, propagating the panic, +/// or in case the thread is intended to be a subsystem boundary +/// that is supposed to isolate system-level failures, +/// match for the `Err` variant and handle the panic in an appropriate way. +/// /// A thread that completes without panicking is considered to exit successfully. /// /// # Examples From b1d0a68fd78f79682121559b3f8225fd56fa1440 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 13:22:43 +0100 Subject: [PATCH 12/29] fix link to ptr docs --- src/libcore/slice/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index a7b759f523103..7655b2f806532 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -5311,7 +5311,7 @@ unsafe impl<'a, T> TrustedRandomAccess for RChunksExactMut<'a, T> { /// assert_eq!(slice[0], 42); /// ``` /// -/// [valid]: ../ptr/index.html#safety +/// [valid]: ../../std/ptr/index.html#safety /// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling /// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset #[inline] @@ -5348,7 +5348,7 @@ pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] { /// * The total size `len * mem::size_of::()` of the slice must be no larger than `isize::MAX`. /// See the safety documentation of [`pointer::offset`]. /// -/// [valid]: ../ptr/index.html#safety +/// [valid]: ../../std/ptr/index.html#safety /// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling /// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset /// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html From 3a54ab78efc66c17a6d2bb57463f5dac4696c7ed Mon Sep 17 00:00:00 2001 From: Oleg Nosov Date: Tue, 5 Nov 2019 15:22:03 +0300 Subject: [PATCH 13/29] LinkedList: PhantomData>> => PhantomData --- src/liballoc/collections/linked_list.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/liballoc/collections/linked_list.rs b/src/liballoc/collections/linked_list.rs index 702df250999fb..da06203a2cbec 100644 --- a/src/liballoc/collections/linked_list.rs +++ b/src/liballoc/collections/linked_list.rs @@ -39,7 +39,7 @@ pub struct LinkedList { head: Option>>, tail: Option>>, len: usize, - marker: PhantomData>>, + marker: PhantomData, } struct Node { @@ -60,7 +60,7 @@ pub struct Iter<'a, T: 'a> { head: Option>>, tail: Option>>, len: usize, - marker: PhantomData<&'a Node>, + marker: PhantomData<&'a T>, } #[stable(feature = "collection_debug", since = "1.17.0")] @@ -90,7 +90,7 @@ impl Clone for Iter<'_, T> { #[stable(feature = "rust1", since = "1.0.0")] pub struct IterMut<'a, T: 'a> { // We do *not* exclusively own the entire list here, references to node's `element` - // have been handed out by the iterator! So be careful when using this; the methods + // have been handed out by the iterator! So be careful when using this; the methods // called must be aware that there can be aliasing pointers to `element`. list: &'a mut LinkedList, head: Option>>, From 45f281d46166fb90c7e9403ad814bebb67aa926d Mon Sep 17 00:00:00 2001 From: Oleg Nosov Date: Tue, 5 Nov 2019 23:34:54 +0300 Subject: [PATCH 14/29] Reverted PhantomData in LinkedList, fixed PhantomData markers in Rc and Arc --- src/liballoc/collections/linked_list.rs | 4 ++-- src/liballoc/rc.rs | 2 +- src/liballoc/sync.rs | 2 +- src/libcore/cell.rs | 4 +++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/liballoc/collections/linked_list.rs b/src/liballoc/collections/linked_list.rs index da06203a2cbec..8d73f352fd4a1 100644 --- a/src/liballoc/collections/linked_list.rs +++ b/src/liballoc/collections/linked_list.rs @@ -39,7 +39,7 @@ pub struct LinkedList { head: Option>>, tail: Option>>, len: usize, - marker: PhantomData, + marker: PhantomData>>, } struct Node { @@ -60,7 +60,7 @@ pub struct Iter<'a, T: 'a> { head: Option>>, tail: Option>>, len: usize, - marker: PhantomData<&'a T>, + marker: PhantomData<&'a Node>, } #[stable(feature = "collection_debug", since = "1.17.0")] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index f1c4c32e116ea..a11f9e8c14579 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -280,7 +280,7 @@ struct RcBox { #[stable(feature = "rust1", since = "1.0.0")] pub struct Rc { ptr: NonNull>, - phantom: PhantomData, + phantom: PhantomData>, } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 80d6c6e0d4390..4b10f089c2950 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -195,7 +195,7 @@ const MAX_REFCOUNT: usize = (isize::MAX) as usize; #[stable(feature = "rust1", since = "1.0.0")] pub struct Arc { ptr: NonNull>, - phantom: PhantomData, + phantom: PhantomData>, } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index fda103a52d8bc..c381998d99a89 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -137,9 +137,11 @@ //! use std::cell::Cell; //! use std::ptr::NonNull; //! use std::intrinsics::abort; +//! use std::marker::PhantomData; //! //! struct Rc { -//! ptr: NonNull> +//! ptr: NonNull>, +//! phantom: PhantomData>, //! } //! //! struct RcBox { From 11a48a0423410376dbe9a6080b41aa90e43cead2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 21:50:55 +0100 Subject: [PATCH 15/29] Apply suggestions from code review Co-Authored-By: Mazdak Farrokhzad --- src/libcore/ptr/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 407f8a6218e4f..1a13ac465f436 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -18,7 +18,7 @@ //! * A [null] pointer is *never* valid, not even for accesses of [size zero][zst]. //! * All pointers (except for the null pointer) are valid for all operations of //! [size zero][zst]. -//! * For a pointer to be valid, it is necessary (but not always sufficient) that the pointer +//! * For a pointer to be valid, it is necessary, but not always sufficient, that the pointer //! be *dereferencable*: the memory range of the given size starting at the pointer must all be //! within the bounds of a single allocated object. Note that in Rust, //! every (stack-allocated) variable is considered a separate allocated object. @@ -253,7 +253,7 @@ pub fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { } /// Performs the same functionality as [`slice_from_raw_parts`], except that a -/// raw mutable slice is returned. +/// raw mutable slice is returned, as opposed to a raw immutable slice. /// /// See the documentation of [`slice_from_raw_parts`] for more details. /// From 6a5931921ca751b8c6748111d15c5d97cb47fab4 Mon Sep 17 00:00:00 2001 From: Oleg Nosov Date: Wed, 6 Nov 2019 01:03:31 +0300 Subject: [PATCH 16/29] Fixed libcore/cell.rs example --- src/libcore/cell.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index c381998d99a89..817c04d0af924 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -153,7 +153,10 @@ //! impl Clone for Rc { //! fn clone(&self) -> Rc { //! self.inc_strong(); -//! Rc { ptr: self.ptr } +//! Rc { +//! ptr: self.ptr, +//! phantom: PhantomData, +//! } //! } //! } //! From f1bc4ef170dc27f7c186e6b00de30f66eee14f6c Mon Sep 17 00:00:00 2001 From: Pyry Kontio Date: Wed, 6 Nov 2019 14:47:52 +0900 Subject: [PATCH 17/29] Addressed review comments. --- src/libstd/thread/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 2af0a4804ef42..5a936da05fd48 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -1273,15 +1273,15 @@ impl fmt::Debug for Thread { /// /// The value contained in the `Result::Err` variant /// is the value the thread panicked with; -/// that is, the parameter the `panic!` macro was called with. +/// that is, the argument the `panic!` macro was called with. /// Unlike with normal errors, this value doesn't implement -/// the `std::error::Error` trait. +/// the [`Error`] trait. /// -/// Thus, a sensible way to handle a thread panic is to either -/// `unwrap` the `Result`, propagating the panic, -/// or in case the thread is intended to be a subsystem boundary +/// Thus, a sensible way to handle a thread panic is to either: +/// 1. `unwrap` the `Result`, propagating the panic +/// 2. or in case the thread is intended to be a subsystem boundary /// that is supposed to isolate system-level failures, -/// match for the `Err` variant and handle the panic in an appropriate way. +/// match on the `Err` variant and handle the panic in an appropriate way. /// /// A thread that completes without panicking is considered to exit successfully. /// From 1c7595fd0f509637e8da61e3bac425e4f3fd69fa Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 25 Oct 2019 07:19:07 +0200 Subject: [PATCH 18/29] gate rustc_on_unimplemented under rustc_attrs --- .../src/language-features/on-unimplemented.md | 154 ------------------ src/liballoc/lib.rs | 2 +- src/libcore/lib.rs | 2 +- src/librustc/error_codes.rs | 6 +- src/libstd/lib.rs | 2 +- src/libsyntax/feature_gate/active.rs | 3 - src/libsyntax/feature_gate/builtin_attrs.rs | 15 +- src/libsyntax/feature_gate/removed.rs | 3 + ...ssue-59523-on-implemented-is-not-unused.rs | 2 +- .../feature-gate-on-unimplemented.rs | 9 - .../ui/on-unimplemented/bad-annotation.rs | 2 +- .../expected-comma-found-token.rs | 2 +- .../feature-gate-on-unimplemented.rs | 8 + .../feature-gate-on-unimplemented.stderr | 8 +- .../ui/on-unimplemented/multiple-impls.rs | 2 +- src/test/ui/on-unimplemented/on-impl.rs | 2 +- src/test/ui/on-unimplemented/on-trait.rs | 2 +- 17 files changed, 34 insertions(+), 190 deletions(-) delete mode 100644 src/doc/unstable-book/src/language-features/on-unimplemented.md delete mode 100644 src/test/ui/feature-gates/feature-gate-on-unimplemented.rs create mode 100644 src/test/ui/on-unimplemented/feature-gate-on-unimplemented.rs rename src/test/ui/{feature-gates => on-unimplemented}/feature-gate-on-unimplemented.stderr (57%) diff --git a/src/doc/unstable-book/src/language-features/on-unimplemented.md b/src/doc/unstable-book/src/language-features/on-unimplemented.md deleted file mode 100644 index 8db241e4b4ebf..0000000000000 --- a/src/doc/unstable-book/src/language-features/on-unimplemented.md +++ /dev/null @@ -1,154 +0,0 @@ -# `on_unimplemented` - -The tracking issue for this feature is: [#29628] - -[#29628]: https://github.com/rust-lang/rust/issues/29628 - ------------------------- - -The `on_unimplemented` feature provides the `#[rustc_on_unimplemented]` -attribute, which allows trait definitions to add specialized notes to error -messages when an implementation was expected but not found. You can refer -to the trait's generic arguments by name and to the resolved type using -`Self`. - -For example: - -```rust,compile_fail -#![feature(on_unimplemented)] - -#[rustc_on_unimplemented="an iterator over elements of type `{A}` \ - cannot be built from a collection of type `{Self}`"] -trait MyIterator { - fn next(&mut self) -> A; -} - -fn iterate_chars>(i: I) { - // ... -} - -fn main() { - iterate_chars(&[1, 2, 3][..]); -} -``` - -When the user compiles this, they will see the following; - -```txt -error[E0277]: the trait bound `&[{integer}]: MyIterator` is not satisfied - --> :14:5 - | -14 | iterate_chars(&[1, 2, 3][..]); - | ^^^^^^^^^^^^^ an iterator over elements of type `char` cannot be built from a collection of type `&[{integer}]` - | - = help: the trait `MyIterator` is not implemented for `&[{integer}]` - = note: required by `iterate_chars` -``` - -`on_unimplemented` also supports advanced filtering for better targeting -of messages, as well as modifying specific parts of the error message. You -target the text of: - - - the main error message (`message`) - - the label (`label`) - - an extra note (`note`) - -For example, the following attribute - -```rust,compile_fail -#[rustc_on_unimplemented( - message="message", - label="label", - note="note" -)] -trait MyIterator { - fn next(&mut self) -> A; -} -``` - -Would generate the following output: - -```text -error[E0277]: message - --> :14:5 - | -14 | iterate_chars(&[1, 2, 3][..]); - | ^^^^^^^^^^^^^ label - | - = note: note - = help: the trait `MyIterator` is not implemented for `&[{integer}]` - = note: required by `iterate_chars` -``` - -To allow more targeted error messages, it is possible to filter the -application of these fields based on a variety of attributes when using -`on`: - - - `crate_local`: whether the code causing the trait bound to not be - fulfilled is part of the user's crate. This is used to avoid suggesting - code changes that would require modifying a dependency. - - Any of the generic arguments that can be substituted in the text can be - referred by name as well for filtering, like `Rhs="i32"`, except for - `Self`. - - `_Self`: to filter only on a particular calculated trait resolution, like - `Self="std::iter::Iterator"`. This is needed because `Self` is a - keyword which cannot appear in attributes. - - `direct`: user-specified rather than derived obligation. - - `from_method`: usable both as boolean (whether the flag is present, like - `crate_local`) or matching against a particular method. Currently used - for `try`. - - `from_desugaring`: usable both as boolean (whether the flag is present) - or matching against a particular desugaring. The desugaring is identified - with its variant name in the `DesugaringKind` enum. - -For example, the `Iterator` trait can be annotated in the following way: - -```rust,compile_fail -#[rustc_on_unimplemented( - on( - _Self="&str", - note="call `.chars()` or `.as_bytes()` on `{Self}" - ), - message="`{Self}` is not an iterator", - label="`{Self}` is not an iterator", - note="maybe try calling `.iter()` or a similar method" -)] -pub trait Iterator {} -``` - -Which would produce the following outputs: - -```text -error[E0277]: `Foo` is not an iterator - --> src/main.rs:4:16 - | -4 | for foo in Foo {} - | ^^^ `Foo` is not an iterator - | - = note: maybe try calling `.iter()` or a similar method - = help: the trait `std::iter::Iterator` is not implemented for `Foo` - = note: required by `std::iter::IntoIterator::into_iter` - -error[E0277]: `&str` is not an iterator - --> src/main.rs:5:16 - | -5 | for foo in "" {} - | ^^ `&str` is not an iterator - | - = note: call `.chars()` or `.bytes() on `&str` - = help: the trait `std::iter::Iterator` is not implemented for `&str` - = note: required by `std::iter::IntoIterator::into_iter` -``` - -If you need to filter on multiple attributes, you can use `all`, `any` or -`not` in the following way: - -```rust,compile_fail -#[rustc_on_unimplemented( - on( - all(_Self="&str", T="std::string::String"), - note="you can coerce a `{T}` into a `{Self}` by writing `&*variable`" - ) -)] -pub trait From: Sized { /* ... */ } -``` diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 94379afc2bd45..ddfa6797a5754 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -116,7 +116,7 @@ #![feature(unsize)] #![feature(unsized_locals)] #![feature(allocator_internals)] -#![feature(on_unimplemented)] +#![cfg_attr(bootstrap, feature(on_unimplemented))] #![feature(rustc_const_unstable)] #![feature(slice_partition_dedup)] #![feature(maybe_uninit_extra, maybe_uninit_slice)] diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index 1b67b05c73021..ca431627147a8 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -89,7 +89,7 @@ #![feature(nll)] #![feature(exhaustive_patterns)] #![feature(no_core)] -#![feature(on_unimplemented)] +#![cfg_attr(bootstrap, feature(on_unimplemented))] #![feature(optin_builtin_traits)] #![feature(prelude_import)] #![feature(repr_simd, platform_intrinsics)] diff --git a/src/librustc/error_codes.rs b/src/librustc/error_codes.rs index f5ff92e69bc7a..18d98efebd42f 100644 --- a/src/librustc/error_codes.rs +++ b/src/librustc/error_codes.rs @@ -607,7 +607,7 @@ position that needs that trait. For example, when the following code is compiled: ```compile_fail -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] fn foo>(x: T){} @@ -639,7 +639,7 @@ position that needs that trait. For example, when the following code is compiled: ```compile_fail -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] fn foo>(x: T){} @@ -669,7 +669,7 @@ position that needs that trait. For example, when the following code is compiled: ```compile_fail -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] fn foo>(x: T){} diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index c7adad896a51a..927fd2a6b0bc6 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -284,7 +284,7 @@ #![feature(never_type)] #![feature(nll)] #![cfg_attr(bootstrap, feature(non_exhaustive))] -#![feature(on_unimplemented)] +#![cfg_attr(bootstrap, feature(on_unimplemented))] #![feature(optin_builtin_traits)] #![feature(panic_info_message)] #![feature(panic_internals)] diff --git a/src/libsyntax/feature_gate/active.rs b/src/libsyntax/feature_gate/active.rs index 736a363bbfc0a..1e77eaaae881d 100644 --- a/src/libsyntax/feature_gate/active.rs +++ b/src/libsyntax/feature_gate/active.rs @@ -134,9 +134,6 @@ declare_features! ( /// Allows using `rustc_*` attributes (RFC 572). (active, rustc_attrs, "1.0.0", Some(29642), None), - /// Allows using `#[on_unimplemented(..)]` on traits. - (active, on_unimplemented, "1.0.0", Some(29628), None), - /// Allows using the `box $expr` syntax. (active, box_syntax, "1.0.0", Some(49733), None), diff --git a/src/libsyntax/feature_gate/builtin_attrs.rs b/src/libsyntax/feature_gate/builtin_attrs.rs index eb811c3e0ff9b..b32a887c6b2a2 100644 --- a/src/libsyntax/feature_gate/builtin_attrs.rs +++ b/src/libsyntax/feature_gate/builtin_attrs.rs @@ -166,7 +166,7 @@ macro_rules! experimental { } const IMPL_DETAIL: &str = "internal implementation detail"; -const INTERAL_UNSTABLE: &str = "this is an internal attribute that will never be stable"; +const INTERNAL_UNSTABLE: &str = "this is an internal attribute that will never be stable"; pub type BuiltinAttribute = (Symbol, AttributeType, AttributeTemplate, AttributeGate); @@ -418,14 +418,14 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ linkage, Whitelisted, template!(NameValueStr: "external|internal|..."), "the `linkage` attribute is experimental and not portable across platforms", ), - rustc_attr!(rustc_std_internal_symbol, Whitelisted, template!(Word), INTERAL_UNSTABLE), + rustc_attr!(rustc_std_internal_symbol, Whitelisted, template!(Word), INTERNAL_UNSTABLE), // ========================================================================== // Internal attributes, Macro related: // ========================================================================== rustc_attr!(rustc_builtin_macro, Whitelisted, template!(Word), IMPL_DETAIL), - rustc_attr!(rustc_proc_macro_decls, Normal, template!(Word), INTERAL_UNSTABLE), + rustc_attr!(rustc_proc_macro_decls, Normal, template!(Word), INTERNAL_UNSTABLE), rustc_attr!( rustc_macro_transparency, Whitelisted, template!(NameValueStr: "transparent|semitransparent|opaque"), @@ -436,17 +436,16 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ // Internal attributes, Diagnostics related: // ========================================================================== - gated!( + rustc_attr!( rustc_on_unimplemented, Whitelisted, template!( List: r#"/*opt*/ message = "...", /*opt*/ label = "...", /*opt*/ note = "...""#, NameValueStr: "message" ), - on_unimplemented, - experimental!(rustc_on_unimplemented), + INTERNAL_UNSTABLE ), // Whitelists "identity-like" conversion methods to suggest on type mismatch. - rustc_attr!(rustc_conversion_suggestion, Whitelisted, template!(Word), INTERAL_UNSTABLE), + rustc_attr!(rustc_conversion_suggestion, Whitelisted, template!(Word), INTERNAL_UNSTABLE), // ========================================================================== // Internal attributes, Const related: @@ -454,7 +453,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_attr!(rustc_promotable, Whitelisted, template!(Word), IMPL_DETAIL), rustc_attr!(rustc_allow_const_fn_ptr, Whitelisted, template!(Word), IMPL_DETAIL), - rustc_attr!(rustc_args_required_const, Whitelisted, template!(List: "N"), INTERAL_UNSTABLE), + rustc_attr!(rustc_args_required_const, Whitelisted, template!(List: "N"), INTERNAL_UNSTABLE), // ========================================================================== // Internal attributes, Layout related: diff --git a/src/libsyntax/feature_gate/removed.rs b/src/libsyntax/feature_gate/removed.rs index 2c29e1ebf1493..c7b931a6f7021 100644 --- a/src/libsyntax/feature_gate/removed.rs +++ b/src/libsyntax/feature_gate/removed.rs @@ -99,6 +99,9 @@ declare_features! ( /// + `__register_diagnostic` /// +`__build_diagnostic_array` (removed, rustc_diagnostic_macros, "1.38.0", None, None, None), + /// Allows using `#[on_unimplemented(..)]` on traits. + /// (Moved to `rustc_attrs`.) + (removed, on_unimplemented, "1.40.0", None, None, None), // ------------------------------------------------------------------------- // feature-group-end: removed features diff --git a/src/test/incremental/issue-59523-on-implemented-is-not-unused.rs b/src/test/incremental/issue-59523-on-implemented-is-not-unused.rs index 709e9be663efa..fa52ca90b105f 100644 --- a/src/test/incremental/issue-59523-on-implemented-is-not-unused.rs +++ b/src/test/incremental/issue-59523-on-implemented-is-not-unused.rs @@ -5,7 +5,7 @@ // revisions: cfail1 cfail2 // build-pass (FIXME(62277): could be check-pass?) -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] #![deny(unused_attributes)] #[rustc_on_unimplemented = "invalid"] diff --git a/src/test/ui/feature-gates/feature-gate-on-unimplemented.rs b/src/test/ui/feature-gates/feature-gate-on-unimplemented.rs deleted file mode 100644 index bec1531c5338f..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-on-unimplemented.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Test that `#[rustc_on_unimplemented]` is gated by `on_unimplemented` feature -// gate. - -#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}`"] -//~^ ERROR the `#[rustc_on_unimplemented]` attribute is an experimental feature -trait Foo -{} - -fn main() {} diff --git a/src/test/ui/on-unimplemented/bad-annotation.rs b/src/test/ui/on-unimplemented/bad-annotation.rs index 5357c3bff9a8a..f05436b8c048a 100644 --- a/src/test/ui/on-unimplemented/bad-annotation.rs +++ b/src/test/ui/on-unimplemented/bad-annotation.rs @@ -1,6 +1,6 @@ // ignore-tidy-linelength -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] #![allow(unused)] diff --git a/src/test/ui/on-unimplemented/expected-comma-found-token.rs b/src/test/ui/on-unimplemented/expected-comma-found-token.rs index d8717f360e9d2..77c0ea17269f0 100644 --- a/src/test/ui/on-unimplemented/expected-comma-found-token.rs +++ b/src/test/ui/on-unimplemented/expected-comma-found-token.rs @@ -2,7 +2,7 @@ // access to the variable, whether that mutable access be used // for direct assignment or for taking mutable ref. Issue #6801. -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] #[rustc_on_unimplemented( message="the message" diff --git a/src/test/ui/on-unimplemented/feature-gate-on-unimplemented.rs b/src/test/ui/on-unimplemented/feature-gate-on-unimplemented.rs new file mode 100644 index 0000000000000..3cc50e3499a09 --- /dev/null +++ b/src/test/ui/on-unimplemented/feature-gate-on-unimplemented.rs @@ -0,0 +1,8 @@ +// Test that `#[rustc_on_unimplemented]` is gated by `rustc_attrs` feature gate. + +#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}`"] +//~^ ERROR this is an internal attribute that will never be stable +trait Foo +{} + +fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-on-unimplemented.stderr b/src/test/ui/on-unimplemented/feature-gate-on-unimplemented.stderr similarity index 57% rename from src/test/ui/feature-gates/feature-gate-on-unimplemented.stderr rename to src/test/ui/on-unimplemented/feature-gate-on-unimplemented.stderr index 6c230f8cada8b..ec1eaff52bd7d 100644 --- a/src/test/ui/feature-gates/feature-gate-on-unimplemented.stderr +++ b/src/test/ui/on-unimplemented/feature-gate-on-unimplemented.stderr @@ -1,11 +1,11 @@ -error[E0658]: the `#[rustc_on_unimplemented]` attribute is an experimental feature - --> $DIR/feature-gate-on-unimplemented.rs:4:1 +error[E0658]: this is an internal attribute that will never be stable + --> $DIR/feature-gate-on-unimplemented.rs:3:1 | LL | #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}`"] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: for more information, see https://github.com/rust-lang/rust/issues/29628 - = help: add `#![feature(on_unimplemented)]` to the crate attributes to enable + = note: for more information, see https://github.com/rust-lang/rust/issues/29642 + = help: add `#![feature(rustc_attrs)]` to the crate attributes to enable error: aborting due to previous error diff --git a/src/test/ui/on-unimplemented/multiple-impls.rs b/src/test/ui/on-unimplemented/multiple-impls.rs index 0aee98b209044..b74957ebcd406 100644 --- a/src/test/ui/on-unimplemented/multiple-impls.rs +++ b/src/test/ui/on-unimplemented/multiple-impls.rs @@ -1,6 +1,6 @@ // Test if the on_unimplemented message override works -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] struct Foo(T); diff --git a/src/test/ui/on-unimplemented/on-impl.rs b/src/test/ui/on-unimplemented/on-impl.rs index 9e4c2f6edd775..ab3e67d01fe44 100644 --- a/src/test/ui/on-unimplemented/on-impl.rs +++ b/src/test/ui/on-unimplemented/on-impl.rs @@ -1,6 +1,6 @@ // Test if the on_unimplemented message override works -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] #[rustc_on_unimplemented = "invalid"] diff --git a/src/test/ui/on-unimplemented/on-trait.rs b/src/test/ui/on-unimplemented/on-trait.rs index 109cb5ba96942..556813cd4795f 100644 --- a/src/test/ui/on-unimplemented/on-trait.rs +++ b/src/test/ui/on-unimplemented/on-trait.rs @@ -1,6 +1,6 @@ // ignore-tidy-linelength -#![feature(on_unimplemented)] +#![feature(rustc_attrs)] pub mod Bar { #[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{Foo}`"] From 4317263a311ad4adc54529b5fdcfe1e18fb651be Mon Sep 17 00:00:00 2001 From: Pyry Kontio Date: Wed, 6 Nov 2019 16:57:59 +0900 Subject: [PATCH 19/29] Fix the Error linking. --- src/libstd/thread/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 5a936da05fd48..a95ebb00714cb 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -1275,7 +1275,7 @@ impl fmt::Debug for Thread { /// is the value the thread panicked with; /// that is, the argument the `panic!` macro was called with. /// Unlike with normal errors, this value doesn't implement -/// the [`Error`] trait. +/// the [`Error`](std::error::Error) trait. /// /// Thus, a sensible way to handle a thread panic is to either: /// 1. `unwrap` the `Result`, propagating the panic From 936349c81b59581c7c3a834a6b61eb94168e2807 Mon Sep 17 00:00:00 2001 From: 3442853561 <21147967+3442853561@users.noreply.github.com> Date: Wed, 6 Nov 2019 16:39:48 +0800 Subject: [PATCH 20/29] Update local.rs Removed parameters not used in the macro --- src/libstd/thread/local.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index cfaab4e22e9cf..46453b47fca8d 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -149,7 +149,7 @@ macro_rules! thread_local { #[allow_internal_unstable(thread_local_internals, cfg_target_thread_local, thread_local)] #[allow_internal_unsafe] macro_rules! __thread_local_inner { - (@key $(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $init:expr) => { + (@key $t:ty, $init:expr) => { { #[inline] fn __init() -> $t { $init } @@ -184,7 +184,7 @@ macro_rules! __thread_local_inner { }; ($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $init:expr) => { $(#[$attr])* $vis const $name: $crate::thread::LocalKey<$t> = - $crate::__thread_local_inner!(@key $(#[$attr])* $vis $name, $t, $init); + $crate::__thread_local_inner!(@key $t, $init); } } From 14ee66acecc353c0b3045d74fbe3addd66f96d79 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 23:04:17 +0100 Subject: [PATCH 21/29] miri cast: avoid unnecessary to_scalar_ptr --- src/librustc_mir/interpret/cast.rs | 4 ++-- src/librustc_mir/interpret/place.rs | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 9ab347957f97a..2037305580ad3 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -260,7 +260,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match (&src_pointee_ty.kind, &dest_pointee_ty.kind) { (&ty::Array(_, length), &ty::Slice(_)) => { - let ptr = self.read_immediate(src)?.to_scalar_ptr()?; + let ptr = self.read_immediate(src)?.to_scalar()?; // u64 cast is from usize to u64, which is always good let val = Immediate::new_slice( ptr, @@ -279,7 +279,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (_, &ty::Dynamic(ref data, _)) => { // Initial cast from sized to dyn trait let vtable = self.get_vtable(src_pointee_ty, data.principal())?; - let ptr = self.read_immediate(src)?.to_scalar_ptr()?; + let ptr = self.read_immediate(src)?.to_scalar()?; let val = Immediate::new_dyn_trait(ptr, vtable); self.write_immediate(val, dest) } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 0289c52fd3744..d696a595777a4 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -287,7 +287,9 @@ where &self, val: ImmTy<'tcx, M::PointerTag>, ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - let pointee_type = val.layout.ty.builtin_deref(true).unwrap().ty; + let pointee_type = val.layout.ty.builtin_deref(true) + .expect("`ref_to_mplace` called on non-ptr type") + .ty; let layout = self.layout_of(pointee_type)?; let mplace = MemPlace { From 32453ce488909ec12b893ceb9a204718eae724e4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Nov 2019 08:44:15 +0100 Subject: [PATCH 22/29] remvoe to_scalar_ptr and use ref_to_mplace everywhere --- src/librustc_mir/interpret/intern.rs | 22 ++++++-------- src/librustc_mir/interpret/operand.rs | 20 ------------ src/librustc_mir/interpret/place.rs | 8 +++-- src/librustc_mir/interpret/validity.rs | 42 +++++++++----------------- 4 files changed, 30 insertions(+), 62 deletions(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 924529d7f5579..2171ceaa452c8 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -192,20 +192,18 @@ for let ty = mplace.layout.ty; if let ty::Ref(_, referenced_ty, mutability) = ty.kind { let value = self.ecx.read_immediate(mplace.into())?; + let mplace = self.ecx.ref_to_mplace(value)?; // Handle trait object vtables - if let Ok(meta) = value.to_meta() { - if let ty::Dynamic(..) = - self.ecx.tcx.struct_tail_erasing_lifetimes( - referenced_ty, self.ecx.param_env).kind - { - if let Ok(vtable) = meta.unwrap().to_ptr() { - // explitly choose `Immutable` here, since vtables are immutable, even - // if the reference of the fat pointer is mutable - self.intern_shallow(vtable.alloc_id, Mutability::Immutable, None)?; - } + if let ty::Dynamic(..) = + self.ecx.tcx.struct_tail_erasing_lifetimes( + referenced_ty, self.ecx.param_env).kind + { + if let Ok(vtable) = mplace.meta.unwrap().to_ptr() { + // explitly choose `Immutable` here, since vtables are immutable, even + // if the reference of the fat pointer is mutable + self.intern_shallow(vtable.alloc_id, Mutability::Immutable, None)?; } } - let mplace = self.ecx.ref_to_mplace(value)?; // Check if we have encountered this pointer+layout combination before. // Only recurse for allocation-backed pointers. if let Scalar::Ptr(ptr) = mplace.ptr { @@ -230,7 +228,7 @@ for ty::Array(_, n) if n.eval_usize(self.ecx.tcx.tcx, self.ecx.param_env) == 0 => {} ty::Slice(_) - if value.to_meta().unwrap().unwrap().to_usize(self.ecx)? == 0 => {} + if mplace.meta.unwrap().to_usize(self.ecx)? == 0 => {} _ => bug!("const qualif failed to prevent mutable references"), } }, diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index ae23971849eea..d80ad3848d20a 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -82,26 +82,6 @@ impl<'tcx, Tag> Immediate { Immediate::ScalarPair(a, b) => Ok((a.not_undef()?, b.not_undef()?)) } } - - /// Converts the immediate into a pointer (or a pointer-sized integer). - /// Throws away the second half of a ScalarPair! - #[inline] - pub fn to_scalar_ptr(self) -> InterpResult<'tcx, Scalar> { - match self { - Immediate::Scalar(ptr) | - Immediate::ScalarPair(ptr, _) => ptr.not_undef(), - } - } - - /// Converts the value into its metadata. - /// Throws away the first half of a ScalarPair! - #[inline] - pub fn to_meta(self) -> InterpResult<'tcx, Option>> { - Ok(match self { - Immediate::Scalar(_) => None, - Immediate::ScalarPair(_, meta) => Some(meta.not_undef()?), - }) - } } // ScalarPair needs a type to interpret, so we often have an immediate and a type together diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index d696a595777a4..36e58d356d100 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -291,15 +291,19 @@ where .expect("`ref_to_mplace` called on non-ptr type") .ty; let layout = self.layout_of(pointee_type)?; + let (ptr, meta) = match *val { + Immediate::Scalar(ptr) => (ptr.not_undef()?, None), + Immediate::ScalarPair(ptr, meta) => (ptr.not_undef()?, Some(meta.not_undef()?)), + }; let mplace = MemPlace { - ptr: val.to_scalar_ptr()?, + ptr, // We could use the run-time alignment here. For now, we do not, because // the point of tracking the alignment here is to make sure that the *static* // alignment information emitted with the loads is correct. The run-time // alignment can only be more restrictive. align: layout.align.abi, - meta: val.to_meta()?, + meta, }; Ok(MPlaceTy { mplace, layout }) } diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 82b8b28d72b7b..929425afc9e91 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -388,44 +388,31 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } } ty::RawPtr(..) => { - // Check pointer part. - if self.ref_tracking_for_consts.is_some() { - // Integers/floats in CTFE: For consistency with integers, we do not - // accept undef. - let _ptr = try_validation!(value.to_scalar_ptr(), - "undefined address in raw pointer", self.path); - } else { - // Remain consistent with `usize`: Accept anything. - } - - // Check metadata. - let meta = try_validation!(value.to_meta(), - "uninitialized data in wide pointer metadata", self.path); - let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; - if layout.is_unsized() { - self.check_wide_ptr_meta(meta, layout)?; + // We are conservative with undef for integers, but try to + // actually enforce our current rules for raw pointers. + let place = try_validation!(self.ecx.ref_to_mplace(value), + "undefined pointer", self.path); + if place.layout.is_unsized() { + self.check_wide_ptr_meta(place.meta, place.layout)?; } } _ if ty.is_box() || ty.is_region_ptr() => { // Handle wide pointers. // Check metadata early, for better diagnostics - let ptr = try_validation!(value.to_scalar_ptr(), - "undefined address in pointer", self.path); - let meta = try_validation!(value.to_meta(), - "uninitialized data in wide pointer metadata", self.path); - let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; - if layout.is_unsized() { - self.check_wide_ptr_meta(meta, layout)?; + let place = try_validation!(self.ecx.ref_to_mplace(value), + "undefined pointer", self.path); + if place.layout.is_unsized() { + self.check_wide_ptr_meta(place.meta, place.layout)?; } // Make sure this is dereferencable and all. - let (size, align) = self.ecx.size_and_align_of(meta, layout)? + let (size, align) = self.ecx.size_and_align_of(place.meta, place.layout)? // for the purpose of validity, consider foreign types to have // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). - .unwrap_or_else(|| (layout.size, layout.align.abi)); + .unwrap_or_else(|| (place.layout.size, place.layout.align.abi)); let ptr: Option<_> = match self.ecx.memory.check_ptr_access_align( - ptr, + place.ptr, size, Some(align), CheckInAllocMsg::InboundsTest, @@ -435,7 +422,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> Err(err) => { info!( "{:?} did not pass access check for size {:?}, align {:?}", - ptr, size, align + place.ptr, size, align ); match err.kind { err_unsup!(InvalidNullPointerUsage) => @@ -459,7 +446,6 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> }; // Recursive checking if let Some(ref mut ref_tracking) = self.ref_tracking_for_consts { - let place = self.ecx.ref_to_mplace(value)?; if let Some(ptr) = ptr { // not a ZST // Skip validation entirely for some external statics let alloc_kind = self.ecx.tcx.alloc_map.lock().get(ptr.alloc_id); From 87edcf095da6218d05639dd62daeda0d0642d0c5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Nov 2019 08:45:50 +0100 Subject: [PATCH 23/29] improve a comment --- src/librustc_mir/interpret/validity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 929425afc9e91..8cb2f6c3462cc 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -613,7 +613,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // reject it. However, that's good: We don't inherently want // to reject those pointers, we just do not have the machinery to // talk about parts of a pointer. - // We also accept undef, for consistency with the type-based checks. + // We also accept undef, for consistency with the slow path. match self.ecx.memory.get(ptr.alloc_id)?.check_bytes( self.ecx, ptr, From 2312a56f5c4e65df4b6d8128489bc58a79555718 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Nov 2019 09:52:11 +0100 Subject: [PATCH 24/29] --bless --- src/test/ui/consts/const-eval/ub-wide-ptr.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/consts/const-eval/ub-wide-ptr.stderr b/src/test/ui/consts/const-eval/ub-wide-ptr.stderr index 9134ef5a31ad9..85fb8ac2a4a36 100644 --- a/src/test/ui/consts/const-eval/ub-wide-ptr.stderr +++ b/src/test/ui/consts/const-eval/ub-wide-ptr.stderr @@ -42,7 +42,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:107:1 | LL | const SLICE_LENGTH_UNINIT: &[u8] = unsafe { SliceTransmute { addr: 42 }.slice}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized data in wide pointer metadata + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered undefined pointer | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. @@ -90,7 +90,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-wide-ptr.rs:133:1 | LL | const RAW_SLICE_LENGTH_UNINIT: *const [u8] = unsafe { SliceTransmute { addr: 42 }.raw_slice}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized data in wide pointer metadata + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered undefined pointer | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. From f2ed1e661e69dc463aa18ef3912501e1dde936b8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Nov 2019 11:15:30 +0100 Subject: [PATCH 25/29] Fix markdown link Co-Authored-By: Oliver Scherer --- src/librustc/hir/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index cbbf808bece06..d6f7ba6b9734e 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1620,7 +1620,7 @@ pub enum ExprKind { /// To resolve the called method to a `DefId`, call [`type_dependent_def_id`] with /// the `hir_id` of the `MethodCall` node itself. /// - /// [`qpath_res`]: ../ty/struct.TypeckTables.html#method.type_dependent_def_id + /// [`type_dependent_def_id`]: ../ty/struct.TypeckTables.html#method.type_dependent_def_id MethodCall(P, Span, HirVec), /// A tuple (e.g., `(a, b, c, d)`). Tup(HirVec), From 2daf7b9fe3b99703076d81319daad619d0f366e7 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 6 Nov 2019 16:41:24 +0100 Subject: [PATCH 26/29] Fix broken link in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c5468a2924888..61d3c9e115720 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ The Rust build system has a Python script called `x.py` to bootstrap building the compiler. More information about it may be found by running `./x.py --help` or reading the [rustc guide][rustcguidebuild]. -[rustcguidebuild]: https://rust-lang.github.io/rustc-guide/how-to-build-and-run.html +[rustcguidebuild]: https://rust-lang.github.io/rustc-guide/building/how-to-build-and-run.html ### Building on *nix 1. Make sure you have installed the dependencies: From 8568204f4e934ff93817f948b13a9c804277a6bf Mon Sep 17 00:00:00 2001 From: Pyry Kontio Date: Thu, 7 Nov 2019 01:45:30 +0900 Subject: [PATCH 27/29] Try with crate::error::Error --- src/libstd/thread/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index a95ebb00714cb..0c632d2afbdd4 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -1275,7 +1275,7 @@ impl fmt::Debug for Thread { /// is the value the thread panicked with; /// that is, the argument the `panic!` macro was called with. /// Unlike with normal errors, this value doesn't implement -/// the [`Error`](std::error::Error) trait. +/// the [`Error`](crate::error::Error) trait. /// /// Thus, a sensible way to handle a thread panic is to either: /// 1. `unwrap` the `Result`, propagating the panic From 1d2a314ef5d0337386b7a6f45eaccba5ef898623 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 7 Nov 2019 06:48:28 +0900 Subject: [PATCH 28/29] Update link on CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 37a217d2a0452..bd1d49fb24dc3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -105,7 +105,7 @@ contributions to the compiler and the standard library. It also lists some really useful commands to the build system (`./x.py`), which could save you a lot of time. -[rustcguidebuild]: https://rust-lang.github.io/rustc-guide/how-to-build-and-run.html +[rustcguidebuild]: https://rust-lang.github.io/rustc-guide/building/how-to-build-and-run.html ## Pull Requests [pull-requests]: #pull-requests From 90b094a01a43f8fc676cb1151ec3994ea02582c7 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 7 Nov 2019 08:30:44 +0900 Subject: [PATCH 29/29] Fix other broken link --- src/librustc/infer/region_constraints/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/infer/region_constraints/README.md b/src/librustc/infer/region_constraints/README.md index ea7fffe9dc1c6..d789fb0de1093 100644 --- a/src/librustc/infer/region_constraints/README.md +++ b/src/librustc/infer/region_constraints/README.md @@ -1,3 +1,3 @@ For info on how the current borrowck works, see the [rustc guide]. -[rustc guide]: https://rust-lang.github.io/rustc-guide/mir/borrowck.html +[rustc guide]: https://rust-lang.github.io/rustc-guide/borrow_check.html