From 65153710e1cc46a33b65f90cd92732bc9c77cee5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Nov 2019 09:50:59 +0100 Subject: [PATCH 01/19] 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 151e9890f4cf8ea714dd6a4760589b322dab567b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Nov 2019 10:16:16 +0100 Subject: [PATCH 02/19] 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 03/19] 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 04/19] 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 05/19] 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 06/19] 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 b1d0a68fd78f79682121559b3f8225fd56fa1440 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Nov 2019 13:22:43 +0100 Subject: [PATCH 07/19] 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 08/19] 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 09/19] 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 10/19] 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 11/19] 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 1c7595fd0f509637e8da61e3bac425e4f3fd69fa Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 25 Oct 2019 07:19:07 +0200 Subject: [PATCH 12/19] 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 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 13/19] 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 14/19] 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 15/19] 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 16/19] 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 17/19] --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 18/19] 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 8ec83cf3597d0c7ac8e7a379f646968af115ebe7 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 6 Nov 2019 14:47:55 +0000 Subject: [PATCH 19/19] update clippy to fix toolstate --- Cargo.lock | 2 +- src/tools/clippy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6dca2048d4e8..0c7b1b740492b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -487,7 +487,7 @@ dependencies = [ "regex-syntax", "semver", "serde", - "smallvec 0.6.10", + "smallvec 1.0.0", "toml", "unicode-normalization", "url 2.1.0", diff --git a/src/tools/clippy b/src/tools/clippy index c8e3cfbdd9978..865f5c7c46a98 160000 --- a/src/tools/clippy +++ b/src/tools/clippy @@ -1 +1 @@ -Subproject commit c8e3cfbdd997839c771ca32c7ac860fe95149a04 +Subproject commit 865f5c7c46a9822c10b5e77340be81872128d6fd