From 9a2bca6a52158577817f0cc1f13c1b48206bf275 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Thu, 16 Apr 2020 02:00:23 +0200 Subject: [PATCH 01/14] rustc: fix check_attr() for methods, closures and foreign functions UI tests are updated with additional error messages that were missing before. --- src/librustc_passes/check_attr.rs | 5 +- ...ture-gate-unboxed-closures-manual-impls.rs | 4 ++ ...-gate-unboxed-closures-manual-impls.stderr | 48 +++++++++++++++---- .../feature-gate-unboxed-closures.rs | 1 + .../feature-gate-unboxed-closures.stderr | 12 ++++- src/test/ui/issues/issue-3214.rs | 3 ++ src/test/ui/issues/issue-3214.stderr | 14 ++++-- src/test/ui/macros/issue-68060.rs | 2 - src/test/ui/macros/issue-68060.stderr | 6 +-- 9 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/librustc_passes/check_attr.rs b/src/librustc_passes/check_attr.rs index 6c9d25cfaa54b..b1b58b0e3532b 100644 --- a/src/librustc_passes/check_attr.rs +++ b/src/librustc_passes/check_attr.rs @@ -76,7 +76,7 @@ impl CheckAttrVisitor<'tcx> { return; } - if target == Target::Fn { + if matches!(target, Target::Fn | Target::Method(_) | Target::ForeignFn) { self.tcx.codegen_fn_attrs(self.tcx.hir().local_def_id(hir_id)); } @@ -391,6 +391,9 @@ impl CheckAttrVisitor<'tcx> { ); } } + if target == Target::Closure { + self.tcx.codegen_fn_attrs(self.tcx.hir().local_def_id(expr.hir_id)); + } } fn check_used(&self, attrs: &'hir [Attribute], target: Target) { diff --git a/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.rs b/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.rs index ff6e2b8290389..eecf2046ccbea 100644 --- a/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.rs +++ b/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.rs @@ -8,24 +8,28 @@ struct Foo; impl Fn<()> for Foo { //~^ ERROR the precise format of `Fn`-family traits' type parameters is subject to change +//~| ERROR manual implementations of `Fn` are experimental extern "rust-call" fn call(self, args: ()) -> () {} //~^ ERROR rust-call ABI is subject to change } struct Foo1; impl FnOnce() for Foo1 { //~^ ERROR associated type bindings are not allowed here +//~| ERROR manual implementations of `FnOnce` are experimental extern "rust-call" fn call_once(self, args: ()) -> () {} //~^ ERROR rust-call ABI is subject to change } struct Bar; impl FnMut<()> for Bar { //~^ ERROR the precise format of `Fn`-family traits' type parameters is subject to change +//~| ERROR manual implementations of `FnMut` are experimental extern "rust-call" fn call_mut(&self, args: ()) -> () {} //~^ ERROR rust-call ABI is subject to change } struct Baz; impl FnOnce<()> for Baz { //~^ ERROR the precise format of `Fn`-family traits' type parameters is subject to change +//~| ERROR manual implementations of `FnOnce` are experimental extern "rust-call" fn call_once(&self, args: ()) -> () {} //~^ ERROR rust-call ABI is subject to change } diff --git a/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr b/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr index 892020332d702..22a1ce3061889 100644 --- a/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr +++ b/src/test/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr @@ -1,5 +1,5 @@ error[E0658]: rust-call ABI is subject to change - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:11:12 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:12:12 | LL | extern "rust-call" fn call(self, args: ()) -> () {} | ^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | extern "rust-call" fn call(self, args: ()) -> () {} = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable error[E0658]: rust-call ABI is subject to change - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:17:12 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:19:12 | LL | extern "rust-call" fn call_once(self, args: ()) -> () {} | ^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | extern "rust-call" fn call_once(self, args: ()) -> () {} = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable error[E0658]: rust-call ABI is subject to change - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:23:12 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:26:12 | LL | extern "rust-call" fn call_mut(&self, args: ()) -> () {} | ^^^^^^^^^^^ @@ -26,7 +26,7 @@ LL | extern "rust-call" fn call_mut(&self, args: ()) -> () {} = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable error[E0658]: rust-call ABI is subject to change - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:29:12 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:33:12 | LL | extern "rust-call" fn call_once(&self, args: ()) -> () {} | ^^^^^^^^^^^ @@ -44,13 +44,13 @@ LL | impl Fn<()> for Foo { = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable error[E0229]: associated type bindings are not allowed here - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:15:6 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:16:6 | LL | impl FnOnce() for Foo1 { | ^^^^^^^^ associated type not allowed here error[E0658]: the precise format of `Fn`-family traits' type parameters is subject to change - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:21:6 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:23:6 | LL | impl FnMut<()> for Bar { | ^^^^^^^^^ help: use parenthetical notation instead: `FnMut() -> ()` @@ -59,7 +59,7 @@ LL | impl FnMut<()> for Bar { = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable error[E0658]: the precise format of `Fn`-family traits' type parameters is subject to change - --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:27:6 + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:30:6 | LL | impl FnOnce<()> for Baz { | ^^^^^^^^^^ help: use parenthetical notation instead: `FnOnce() -> ()` @@ -67,7 +67,39 @@ LL | impl FnOnce<()> for Baz { = note: see issue #29625 for more information = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable -error: aborting due to 8 previous errors +error[E0183]: manual implementations of `Fn` are experimental + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:9:1 + | +LL | impl Fn<()> for Foo { + | ^^^^^^^^^^^^^^^^^^^ manual implementations of `Fn` are experimental + | + = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable + +error[E0183]: manual implementations of `FnMut` are experimental + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:23:1 + | +LL | impl FnMut<()> for Bar { + | ^^^^^^^^^^^^^^^^^^^^^^ manual implementations of `FnMut` are experimental + | + = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable + +error[E0183]: manual implementations of `FnOnce` are experimental + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:16:1 + | +LL | impl FnOnce() for Foo1 { + | ^^^^^^^^^^^^^^^^^^^^^^ manual implementations of `FnOnce` are experimental + | + = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable + +error[E0183]: manual implementations of `FnOnce` are experimental + --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:30:1 + | +LL | impl FnOnce<()> for Baz { + | ^^^^^^^^^^^^^^^^^^^^^^^ manual implementations of `FnOnce` are experimental + | + = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable + +error: aborting due to 12 previous errors Some errors have detailed explanations: E0229, E0658. For more information about an error, try `rustc --explain E0229`. diff --git a/src/test/ui/feature-gates/feature-gate-unboxed-closures.rs b/src/test/ui/feature-gates/feature-gate-unboxed-closures.rs index b8d3aa4a141df..ebc5a2536f672 100644 --- a/src/test/ui/feature-gates/feature-gate-unboxed-closures.rs +++ b/src/test/ui/feature-gates/feature-gate-unboxed-closures.rs @@ -4,6 +4,7 @@ struct Test; impl FnOnce<(u32, u32)> for Test { //~^ ERROR the precise format of `Fn`-family traits' type parameters is subject to change +//~| ERROR manual implementations of `FnOnce` are experimental type Output = u32; extern "rust-call" fn call_once(self, (a, b): (u32, u32)) -> u32 { diff --git a/src/test/ui/feature-gates/feature-gate-unboxed-closures.stderr b/src/test/ui/feature-gates/feature-gate-unboxed-closures.stderr index feda2fe49f95c..2c8915d0ac334 100644 --- a/src/test/ui/feature-gates/feature-gate-unboxed-closures.stderr +++ b/src/test/ui/feature-gates/feature-gate-unboxed-closures.stderr @@ -1,5 +1,5 @@ error[E0658]: rust-call ABI is subject to change - --> $DIR/feature-gate-unboxed-closures.rs:9:12 + --> $DIR/feature-gate-unboxed-closures.rs:10:12 | LL | extern "rust-call" fn call_once(self, (a, b): (u32, u32)) -> u32 { | ^^^^^^^^^^^ @@ -16,6 +16,14 @@ LL | impl FnOnce<(u32, u32)> for Test { = note: see issue #29625 for more information = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable -error: aborting due to 2 previous errors +error[E0183]: manual implementations of `FnOnce` are experimental + --> $DIR/feature-gate-unboxed-closures.rs:5:1 + | +LL | impl FnOnce<(u32, u32)> for Test { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ manual implementations of `FnOnce` are experimental + | + = help: add `#![feature(unboxed_closures)]` to the crate attributes to enable + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/issues/issue-3214.rs b/src/test/ui/issues/issue-3214.rs index 9a727aa305797..030677c879fb5 100644 --- a/src/test/ui/issues/issue-3214.rs +++ b/src/test/ui/issues/issue-3214.rs @@ -1,3 +1,5 @@ +// ignore-tidy-linelength + fn foo() { struct Foo { x: T, //~ ERROR can't use generic parameters from outer function @@ -5,6 +7,7 @@ fn foo() { impl Drop for Foo { //~^ ERROR wrong number of type arguments + //~| ERROR the type parameter `T` is not constrained by the impl trait, self type, or predicates fn drop(&mut self) {} } } diff --git a/src/test/ui/issues/issue-3214.stderr b/src/test/ui/issues/issue-3214.stderr index 02c8da10bb4a3..30bc6cb115ffc 100644 --- a/src/test/ui/issues/issue-3214.stderr +++ b/src/test/ui/issues/issue-3214.stderr @@ -1,5 +1,5 @@ error[E0401]: can't use generic parameters from outer function - --> $DIR/issue-3214.rs:3:12 + --> $DIR/issue-3214.rs:5:12 | LL | fn foo() { | --- - type parameter from outer function @@ -10,12 +10,18 @@ LL | x: T, | ^ use of generic parameter from outer function error[E0107]: wrong number of type arguments: expected 0, found 1 - --> $DIR/issue-3214.rs:6:26 + --> $DIR/issue-3214.rs:8:26 | LL | impl Drop for Foo { | ^ unexpected type argument -error: aborting due to 2 previous errors +error[E0207]: the type parameter `T` is not constrained by the impl trait, self type, or predicates + --> $DIR/issue-3214.rs:8:10 + | +LL | impl Drop for Foo { + | ^ unconstrained type parameter + +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0107, E0401. +Some errors have detailed explanations: E0107, E0207, E0401. For more information about an error, try `rustc --explain E0107`. diff --git a/src/test/ui/macros/issue-68060.rs b/src/test/ui/macros/issue-68060.rs index 85ebd66b66cb6..bc70f8ffec2b2 100644 --- a/src/test/ui/macros/issue-68060.rs +++ b/src/test/ui/macros/issue-68060.rs @@ -1,5 +1,3 @@ -// build-fail - #![feature(track_caller)] fn main() { diff --git a/src/test/ui/macros/issue-68060.stderr b/src/test/ui/macros/issue-68060.stderr index 230867410d966..3ea49e614e633 100644 --- a/src/test/ui/macros/issue-68060.stderr +++ b/src/test/ui/macros/issue-68060.stderr @@ -1,5 +1,5 @@ error: `#[target_feature(..)]` can only be applied to `unsafe` functions - --> $DIR/issue-68060.rs:8:13 + --> $DIR/issue-68060.rs:6:13 | LL | #[target_feature(enable = "")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can only be applied to `unsafe` functions @@ -8,13 +8,13 @@ LL | |_| (), | ------ not an `unsafe` function error: the feature named `` is not valid for this target - --> $DIR/issue-68060.rs:8:30 + --> $DIR/issue-68060.rs:6:30 | LL | #[target_feature(enable = "")] | ^^^^^^^^^^^ `` is not valid for this target error[E0737]: `#[track_caller]` requires Rust ABI - --> $DIR/issue-68060.rs:11:13 + --> $DIR/issue-68060.rs:9:13 | LL | #[track_caller] | ^^^^^^^^^^^^^^^ From 6c700dc11c72993d5fa5905355a69c6524e960d3 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Thu, 16 Apr 2020 17:11:05 +0200 Subject: [PATCH 02/14] Extend UI tests for fixed check_attr() Add testcases for the `#[track_caller]` and `#[target_feature(..)]` function attributes for errors that were not not caught before. --- .../error-with-invalid-abi.rs | 6 ++++++ .../error-with-invalid-abi.stderr | 8 +++++++- .../ui/target-feature/invalid-attribute.rs | 17 ++++++++++++++++ .../target-feature/invalid-attribute.stderr | 20 ++++++++++++++++++- 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs index 20d29619ba404..1145f7786c78b 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs +++ b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs @@ -4,4 +4,10 @@ extern "C" fn f() {} //~^^ ERROR `#[track_caller]` requires Rust ABI +extern "C" { + #[track_caller] + fn g(); + //~^^ ERROR `#[track_caller]` requires Rust ABI +} + fn main() {} diff --git a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr index 2a3a4385c8bf7..e08c52fabd6b7 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr +++ b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr @@ -4,6 +4,12 @@ error[E0737]: `#[track_caller]` requires Rust ABI LL | #[track_caller] | ^^^^^^^^^^^^^^^ -error: aborting due to previous error +error[E0737]: `#[track_caller]` requires Rust ABI + --> $DIR/error-with-invalid-abi.rs:8:5 + | +LL | #[track_caller] + | ^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0737`. diff --git a/src/test/ui/target-feature/invalid-attribute.rs b/src/test/ui/target-feature/invalid-attribute.rs index 46680336632f9..19c8c3dd4886b 100644 --- a/src/test/ui/target-feature/invalid-attribute.rs +++ b/src/test/ui/target-feature/invalid-attribute.rs @@ -65,9 +65,26 @@ trait Baz { } #[target_feature(enable = "sse2")] unsafe fn test() {} +trait Quux { + fn foo(); +} + +impl Quux for Foo { + #[target_feature(enable = "sse2")] + //~^ ERROR `#[target_feature(..)]` can only be applied to `unsafe` functions + //~| NOTE can only be applied to `unsafe` functions + fn foo() {} + //~^ NOTE not an `unsafe` function +} + fn main() { unsafe { foo(); bar(); } + #[target_feature(enable = "sse2")] + //~^ ERROR `#[target_feature(..)]` can only be applied to `unsafe` functions + //~| NOTE can only be applied to `unsafe` functions + || {}; + //~^ NOTE not an `unsafe` function } diff --git a/src/test/ui/target-feature/invalid-attribute.stderr b/src/test/ui/target-feature/invalid-attribute.stderr index abfe5dd219770..76273d66ac2cf 100644 --- a/src/test/ui/target-feature/invalid-attribute.stderr +++ b/src/test/ui/target-feature/invalid-attribute.stderr @@ -91,5 +91,23 @@ error: cannot use `#[inline(always)]` with `#[target_feature]` LL | #[inline(always)] | ^^^^^^^^^^^^^^^^^ -error: aborting due to 12 previous errors +error: `#[target_feature(..)]` can only be applied to `unsafe` functions + --> $DIR/invalid-attribute.rs:85:5 + | +LL | #[target_feature(enable = "sse2")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can only be applied to `unsafe` functions +... +LL | || {}; + | ----- not an `unsafe` function + +error: `#[target_feature(..)]` can only be applied to `unsafe` functions + --> $DIR/invalid-attribute.rs:73:5 + | +LL | #[target_feature(enable = "sse2")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can only be applied to `unsafe` functions +... +LL | fn foo() {} + | ----------- not an `unsafe` function + +error: aborting due to 14 previous errors From e8ffa2182bc41f3511a214a8ae6dad85d9d60e79 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 Apr 2020 23:48:22 +0200 Subject: [PATCH 03/14] better document const-pattern dynamic soundness checks, and fix a soundness hole --- src/librustc_mir/const_eval/eval_queries.rs | 2 +- src/librustc_mir/const_eval/machine.rs | 9 ++++++++- src/librustc_mir/interpret/validity.rs | 22 ++++++++++++++------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/const_eval/eval_queries.rs b/src/librustc_mir/const_eval/eval_queries.rs index 95c5d0f0b1059..d97546e4391b5 100644 --- a/src/librustc_mir/const_eval/eval_queries.rs +++ b/src/librustc_mir/const_eval/eval_queries.rs @@ -193,7 +193,7 @@ fn validate_and_turn_into_const<'tcx>( mplace.into(), path, &mut ref_tracking, - /*may_ref_to_static*/ is_static, + /*may_ref_to_static*/ ecx.memory.extra.can_access_statics, )?; } } diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index 84031ec0f1764..ce9d25599ff9a 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -99,7 +99,12 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> { #[derive(Copy, Clone, Debug)] pub struct MemoryExtra { - /// Whether this machine may read from statics + /// We need to make sure consts never point to anything mutable, even recursively. That is + /// relied on for pattern matching on consts with references. + /// To achieve this, two pieces have to work together: + /// * Interning makes everything outside of statics immutable. + /// * Pointers to allocations inside of statics can never leak outside, to a non-static global. + /// This boolean here controls the second part. pub(super) can_access_statics: bool, } @@ -337,6 +342,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } else if static_def_id.is_some() { // Machine configuration does not allow us to read statics // (e.g., `const` initializer). + // See const_eval::machine::MemoryExtra::can_access_statics for why + // this check is so important. Err(ConstEvalErrKind::ConstAccessesStatic.into()) } else { // Immutable global, this read is fine. diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 838a5b32fc80b..df3c353220318 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -404,19 +404,27 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' // Skip validation entirely for some external statics let alloc_kind = self.ecx.tcx.alloc_map.lock().get(ptr.alloc_id); if let Some(GlobalAlloc::Static(did)) = alloc_kind { - // `extern static` cannot be validated as they have no body. - // FIXME: Statics from other crates are also skipped. - // They might be checked at a different type, but for now we - // want to avoid recursing too deeply. This is not sound! - if !did.is_local() || self.ecx.tcx.is_foreign_item(did) { - return Ok(()); - } + // See const_eval::machine::MemoryExtra::can_access_statics for why + // this check is so important. + // This check is reachable when the const just referenced the static, + // but never read it (so we never entered `before_access_global`). + // We also need to do it here instead of going on to avoid running + // into the `before_access_global` check during validation. if !self.may_ref_to_static && self.ecx.tcx.is_static(did) { throw_validation_failure!( format_args!("a {} pointing to a static variable", kind), self.path ); } + // `extern static` cannot be validated as they have no body. + // FIXME: Statics from other crates are also skipped. + // They might be checked at a different type, but for now we + // want to avoid recursing too deeply. We might miss const-invalid data, + // but things are still sound otherwise (in particular re: consts + // referring to statics). + if !did.is_local() || self.ecx.tcx.is_foreign_item(did) { + return Ok(()); + } } } // Proceed recursively even for ZST, no reason to skip them! From a84e2a0c91613e92ebf946d650b315aa2cd111a1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 Apr 2020 23:49:36 +0200 Subject: [PATCH 04/14] add test for const-ref-to-cross-crate-mutable-static --- .../auxiliary/static_cross_crate.rs | 1 + .../miri_unleashed/const_refers_to_static.rs | 3 +- .../const_refers_to_static.stderr | 16 ++++---- .../miri_unleashed/const_refers_to_static2.rs | 7 +++- .../const_refers_to_static2.stderr | 12 ++++-- .../const_refers_to_static_cross_crate.rs | 35 ++++++++++++++++ .../const_refers_to_static_cross_crate.stderr | 41 +++++++++++++++++++ 7 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs create mode 100644 src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs create mode 100644 src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr diff --git a/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs b/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs new file mode 100644 index 0000000000000..c86b724ac9438 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs @@ -0,0 +1 @@ +pub static mut ZERO: [u8; 1] = [0]; diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static.rs b/src/test/ui/consts/miri_unleashed/const_refers_to_static.rs index 11f4a30d1779b..6b205a2f66d9b 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static.rs +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static.rs @@ -7,7 +7,8 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; -// These tests only cause an error when *using* the const. +// These fail during CTFE (as they read a static), so they only cause an error +// when *using* the const. const MUTATE_INTERIOR_MUT: usize = { static FOO: AtomicUsize = AtomicUsize::new(0); diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static.stderr index 788762808f13b..acc3b637f58bc 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static.stderr @@ -1,47 +1,47 @@ warning: skipping const checks - --> $DIR/const_refers_to_static.rs:14:5 + --> $DIR/const_refers_to_static.rs:15:5 | LL | FOO.fetch_add(1, Ordering::Relaxed) | ^^^ warning: skipping const checks - --> $DIR/const_refers_to_static.rs:14:5 + --> $DIR/const_refers_to_static.rs:15:5 | LL | FOO.fetch_add(1, Ordering::Relaxed) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: skipping const checks - --> $DIR/const_refers_to_static.rs:21:17 + --> $DIR/const_refers_to_static.rs:22:17 | LL | unsafe { *(&FOO as *const _ as *const usize) } | ^^^ warning: skipping const checks - --> $DIR/const_refers_to_static.rs:26:32 + --> $DIR/const_refers_to_static.rs:27:32 | LL | const READ_MUT: u32 = unsafe { MUTABLE }; | ^^^^^^^ warning: skipping const checks - --> $DIR/const_refers_to_static.rs:26:32 + --> $DIR/const_refers_to_static.rs:27:32 | LL | const READ_MUT: u32 = unsafe { MUTABLE }; | ^^^^^^^ error[E0080]: erroneous constant used - --> $DIR/const_refers_to_static.rs:31:5 + --> $DIR/const_refers_to_static.rs:32:5 | LL | MUTATE_INTERIOR_MUT; | ^^^^^^^^^^^^^^^^^^^ referenced constant has errors error[E0080]: erroneous constant used - --> $DIR/const_refers_to_static.rs:33:5 + --> $DIR/const_refers_to_static.rs:34:5 | LL | READ_INTERIOR_MUT; | ^^^^^^^^^^^^^^^^^ referenced constant has errors error[E0080]: erroneous constant used - --> $DIR/const_refers_to_static.rs:35:5 + --> $DIR/const_refers_to_static.rs:36:5 | LL | READ_MUT; | ^^^^^^^^ referenced constant has errors diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.rs b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.rs index 2704f2a7d73c1..553d90f1891cc 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.rs +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.rs @@ -6,9 +6,12 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; -// These tests cause immediate error when *defining* the const. +// These only fail during validation (they do not use but just create a reference to a static), +// so they cause an immediate error when *defining* the const. const REF_INTERIOR_MUT: &usize = { //~ ERROR undefined behavior to use this value +//~| NOTE encountered a reference pointing to a static variable +//~| NOTE static FOO: AtomicUsize = AtomicUsize::new(0); unsafe { &*(&FOO as *const _ as *const usize) } //~^ WARN skipping const checks @@ -16,6 +19,8 @@ const REF_INTERIOR_MUT: &usize = { //~ ERROR undefined behavior to use this valu // ok some day perhaps const READ_IMMUT: &usize = { //~ ERROR it is undefined behavior to use this value +//~| NOTE encountered a reference pointing to a static variable +//~| NOTE static FOO: usize = 0; &FOO //~^ WARN skipping const checks diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.stderr index 2a233d63efef8..33f4a42605cce 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.stderr @@ -1,19 +1,21 @@ warning: skipping const checks - --> $DIR/const_refers_to_static2.rs:13:18 + --> $DIR/const_refers_to_static2.rs:16:18 | LL | unsafe { &*(&FOO as *const _ as *const usize) } | ^^^ warning: skipping const checks - --> $DIR/const_refers_to_static2.rs:20:6 + --> $DIR/const_refers_to_static2.rs:25:6 | LL | &FOO | ^^^ error[E0080]: it is undefined behavior to use this value - --> $DIR/const_refers_to_static2.rs:11:1 + --> $DIR/const_refers_to_static2.rs:12:1 | LL | / const REF_INTERIOR_MUT: &usize = { +LL | | +LL | | LL | | static FOO: AtomicUsize = AtomicUsize::new(0); LL | | unsafe { &*(&FOO as *const _ as *const usize) } LL | | @@ -23,9 +25,11 @@ LL | | }; = 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. error[E0080]: it is undefined behavior to use this value - --> $DIR/const_refers_to_static2.rs:18:1 + --> $DIR/const_refers_to_static2.rs:21:1 | LL | / const READ_IMMUT: &usize = { +LL | | +LL | | LL | | static FOO: usize = 0; LL | | &FOO LL | | diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs new file mode 100644 index 0000000000000..cf277308c0895 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs @@ -0,0 +1,35 @@ +// compile-flags: -Zunleash-the-miri-inside-of-you +// aux-build:static_cross_crate.rs +#![allow(const_err)] + +#![feature(exclusive_range_pattern)] +#![feature(half_open_range_patterns)] + +extern crate static_cross_crate; + +// Sneaky: reference to a mutable static. +// Allowing this would be a disaster for pattern matching, we could violate exhaustiveness checking! +const SLICE_MUT: &[u8; 1] = { //~ ERROR undefined behavior to use this value +//~| NOTE encountered a reference pointing to a static variable +//~| NOTE + unsafe { &static_cross_crate::ZERO } + //~^ WARN skipping const checks + //~| WARN skipping const checks +}; + +pub fn test(x: &[u8; 1]) -> bool { + match x { + SLICE_MUT => true, + //~^ ERROR could not evaluate constant pattern + //~| ERROR could not evaluate constant pattern + &[1..] => false, + } +} + +fn main() { + unsafe { + static_cross_crate::ZERO[0] = 1; + } + // Now the pattern is not exhaustive any more! + test(&[0]); +} diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr new file mode 100644 index 0000000000000..dfee007c74049 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr @@ -0,0 +1,41 @@ +warning: skipping const checks + --> $DIR/const_refers_to_static_cross_crate.rs:15:15 + | +LL | unsafe { &static_cross_crate::ZERO } + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: skipping const checks + --> $DIR/const_refers_to_static_cross_crate.rs:15:15 + | +LL | unsafe { &static_cross_crate::ZERO } + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0080]: it is undefined behavior to use this value + --> $DIR/const_refers_to_static_cross_crate.rs:12:1 + | +LL | / const SLICE_MUT: &[u8; 1] = { +LL | | +LL | | +LL | | unsafe { &static_cross_crate::ZERO } +LL | | +LL | | +LL | | }; + | |__^ type validation failed: encountered a reference pointing to a static variable + | + = 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. + +error: could not evaluate constant pattern + --> $DIR/const_refers_to_static_cross_crate.rs:22:9 + | +LL | SLICE_MUT => true, + | ^^^^^^^^^ + +error: could not evaluate constant pattern + --> $DIR/const_refers_to_static_cross_crate.rs:22:9 + | +LL | SLICE_MUT => true, + | ^^^^^^^^^ + +error: aborting due to 3 previous errors; 2 warnings emitted + +For more information about this error, try `rustc --explain E0080`. From a0898019485fd0920957444b7b387eec5f2b112d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 Apr 2020 23:54:47 +0200 Subject: [PATCH 05/14] clarify comment --- src/librustc_mir/const_eval/machine.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index ce9d25599ff9a..7c1ab261eb9c4 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -343,7 +343,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // Machine configuration does not allow us to read statics // (e.g., `const` initializer). // See const_eval::machine::MemoryExtra::can_access_statics for why - // this check is so important. + // this check is so important: if we could read statics, we could read pointers + // to mutable allocations *inside* statics. These allocations are not themselves + // statics, so pointers to them can get around the check in `validity.rs`. Err(ConstEvalErrKind::ConstAccessesStatic.into()) } else { // Immutable global, this read is fine. From edfca5fe9c7b6c12a9d48ea2dffd6fffff8501a4 Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Mon, 27 Apr 2020 01:41:38 +0800 Subject: [PATCH 06/14] Move branch point upwards to avoid unnecessary mk_ptr() --- src/librustc_typeck/check/coercion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 3d665123f6767..c1a3ff518b338 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -771,10 +771,10 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { ty::RawPtr(mt) => (false, mt), _ => return self.unify_and(a, b, identity), }; + coerce_mutbls(mt_a.mutbl, mutbl_b)?; // Check that the types which they point at are compatible. let a_unsafe = self.tcx.mk_ptr(ty::TypeAndMut { mutbl: mutbl_b, ty: mt_a.ty }); - coerce_mutbls(mt_a.mutbl, mutbl_b)?; // Although references and unsafe ptrs have the same // representation, we still register an Adjust::DerefRef so that // regionck knows that the region for `a` must be valid here. From a9340b1f69b0a301214934cd9b24dcf348522988 Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Tue, 28 Apr 2020 10:58:06 +0800 Subject: [PATCH 07/14] Rename function to `suggest_deref_ref_or_into` because it's suggesting derefence instructions --- src/librustc_typeck/check/demand.rs | 2 +- src/librustc_typeck/check/expr.rs | 2 +- src/librustc_typeck/check/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index c75283e419a6d..8c3a010303768 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -25,7 +25,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) { self.annotate_expected_due_to_let_ty(err, expr); self.suggest_compatible_variants(err, expr, expected, expr_ty); - self.suggest_ref_or_into(err, expr, expected, expr_ty); + self.suggest_deref_ref_or_into(err, expr, expected, expr_ty); if self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) { return; } diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index d287589789e2d..92ddfbff824cd 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -86,7 +86,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) { let expr = expr.peel_drop_temps(); - self.suggest_ref_or_into(&mut err, expr, expected_ty, ty); + self.suggest_deref_ref_or_into(&mut err, expr, expected_ty, ty); extend_err(&mut err); // Error possibly reported in `check_assign` so avoid emitting error again. err.emit_unless(self.is_assign_to_bool(expr, expected_ty)); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 5877c6d269ad6..adbab3d4cb620 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5029,7 +5029,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } - pub fn suggest_ref_or_into( + pub fn suggest_deref_ref_or_into( &self, err: &mut DiagnosticBuilder<'_>, expr: &hir::Expr<'_>, From 979bbf2ce1568f305fb52c2e10dc9db2375ce5bd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 Apr 2020 10:47:16 +0200 Subject: [PATCH 08/14] also test reference into static field --- .../const_refers_to_static_cross_crate.rs | 19 ++++++++-- .../const_refers_to_static_cross_crate.stderr | 36 ++++++++++++------- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs index cf277308c0895..dd4982539174b 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zunleash-the-miri-inside-of-you +// compile-flags: -Zunleash-the-miri-inside-of-you -Zdeduplicate-diagnostics // aux-build:static_cross_crate.rs #![allow(const_err)] @@ -14,18 +14,31 @@ const SLICE_MUT: &[u8; 1] = { //~ ERROR undefined behavior to use this value //~| NOTE unsafe { &static_cross_crate::ZERO } //~^ WARN skipping const checks - //~| WARN skipping const checks +}; + +const SLICE_MUT2: &u8 = { //~ ERROR undefined behavior to use this value +//~| NOTE encountered a reference pointing to a static variable +//~| NOTE + unsafe { &static_cross_crate::ZERO[0] } + //~^ WARN skipping const checks }; pub fn test(x: &[u8; 1]) -> bool { match x { SLICE_MUT => true, //~^ ERROR could not evaluate constant pattern - //~| ERROR could not evaluate constant pattern &[1..] => false, } } +pub fn test2(x: &u8) -> bool { + match x { + SLICE_MUT2 => true, + //~^ ERROR could not evaluate constant pattern + &(1..) => false, + } +} + fn main() { unsafe { static_cross_crate::ZERO[0] = 1; diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr index dfee007c74049..b176dbd2e4d4c 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr @@ -4,12 +4,6 @@ warning: skipping const checks LL | unsafe { &static_cross_crate::ZERO } | ^^^^^^^^^^^^^^^^^^^^^^^^ -warning: skipping const checks - --> $DIR/const_refers_to_static_cross_crate.rs:15:15 - | -LL | unsafe { &static_cross_crate::ZERO } - | ^^^^^^^^^^^^^^^^^^^^^^^^ - error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static_cross_crate.rs:12:1 | @@ -18,24 +12,42 @@ LL | | LL | | LL | | unsafe { &static_cross_crate::ZERO } LL | | -LL | | LL | | }; | |__^ type validation failed: encountered a reference pointing to a static variable | = 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. error: could not evaluate constant pattern - --> $DIR/const_refers_to_static_cross_crate.rs:22:9 + --> $DIR/const_refers_to_static_cross_crate.rs:28:9 | LL | SLICE_MUT => true, | ^^^^^^^^^ +warning: skipping const checks + --> $DIR/const_refers_to_static_cross_crate.rs:22:15 + | +LL | unsafe { &static_cross_crate::ZERO[0] } + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0080]: it is undefined behavior to use this value + --> $DIR/const_refers_to_static_cross_crate.rs:19:1 + | +LL | / const SLICE_MUT2: &u8 = { +LL | | +LL | | +LL | | unsafe { &static_cross_crate::ZERO[0] } +LL | | +LL | | }; + | |__^ type validation failed: encountered a reference pointing to a static variable + | + = 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. + error: could not evaluate constant pattern - --> $DIR/const_refers_to_static_cross_crate.rs:22:9 + --> $DIR/const_refers_to_static_cross_crate.rs:36:9 | -LL | SLICE_MUT => true, - | ^^^^^^^^^ +LL | SLICE_MUT2 => true, + | ^^^^^^^^^^ -error: aborting due to 3 previous errors; 2 warnings emitted +error: aborting due to 4 previous errors; 2 warnings emitted For more information about this error, try `rustc --explain E0080`. From a03355dea0b7d042b8b4e01d75bba6c7489e3a14 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 Apr 2020 14:55:05 +0200 Subject: [PATCH 09/14] some more test cases --- .../auxiliary/static_cross_crate.rs | 2 + .../const_refers_to_static_cross_crate.rs | 41 +++++++++- .../const_refers_to_static_cross_crate.stderr | 80 ++++++++++++++++--- 3 files changed, 109 insertions(+), 14 deletions(-) diff --git a/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs b/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs index c86b724ac9438..4fc6ae66a1242 100644 --- a/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs +++ b/src/test/ui/consts/miri_unleashed/auxiliary/static_cross_crate.rs @@ -1 +1,3 @@ pub static mut ZERO: [u8; 1] = [0]; +pub static ZERO_REF: &[u8; 1] = unsafe { &ZERO }; +pub static mut OPT_ZERO: Option = Some(0); diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs index dd4982539174b..8bdb48e6f122f 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.rs @@ -2,8 +2,7 @@ // aux-build:static_cross_crate.rs #![allow(const_err)] -#![feature(exclusive_range_pattern)] -#![feature(half_open_range_patterns)] +#![feature(exclusive_range_pattern, half_open_range_patterns, const_if_match, const_panic)] extern crate static_cross_crate; @@ -16,13 +15,29 @@ const SLICE_MUT: &[u8; 1] = { //~ ERROR undefined behavior to use this value //~^ WARN skipping const checks }; -const SLICE_MUT2: &u8 = { //~ ERROR undefined behavior to use this value +const U8_MUT: &u8 = { //~ ERROR undefined behavior to use this value //~| NOTE encountered a reference pointing to a static variable //~| NOTE unsafe { &static_cross_crate::ZERO[0] } //~^ WARN skipping const checks }; +// Also test indirection that reads from other static. This causes a const_err. +#[warn(const_err)] //~ NOTE +const U8_MUT2: &u8 = { //~ NOTE + unsafe { &(*static_cross_crate::ZERO_REF)[0] } + //~^ WARN skipping const checks + //~| WARN [const_err] + //~| NOTE constant accesses static +}; +#[warn(const_err)] //~ NOTE +const U8_MUT3: &u8 = { //~ NOTE + unsafe { match static_cross_crate::OPT_ZERO { Some(ref u) => u, None => panic!() } } + //~^ WARN skipping const checks + //~| WARN [const_err] + //~| NOTE constant accesses static +}; + pub fn test(x: &[u8; 1]) -> bool { match x { SLICE_MUT => true, @@ -33,7 +48,24 @@ pub fn test(x: &[u8; 1]) -> bool { pub fn test2(x: &u8) -> bool { match x { - SLICE_MUT2 => true, + U8_MUT => true, + //~^ ERROR could not evaluate constant pattern + &(1..) => false, + } +} + +// We need to use these *in a pattern* to trigger the failure... likely because +// the errors above otherwise stop compilation too early? +pub fn test3(x: &u8) -> bool { + match x { + U8_MUT2 => true, + //~^ ERROR could not evaluate constant pattern + &(1..) => false, + } +} +pub fn test4(x: &u8) -> bool { + match x { + U8_MUT3 => true, //~^ ERROR could not evaluate constant pattern &(1..) => false, } @@ -45,4 +77,5 @@ fn main() { } // Now the pattern is not exhaustive any more! test(&[0]); + test2(&0); } diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr index b176dbd2e4d4c..bc6375f3d5e0b 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.stderr @@ -1,11 +1,11 @@ warning: skipping const checks - --> $DIR/const_refers_to_static_cross_crate.rs:15:15 + --> $DIR/const_refers_to_static_cross_crate.rs:14:15 | LL | unsafe { &static_cross_crate::ZERO } | ^^^^^^^^^^^^^^^^^^^^^^^^ error[E0080]: it is undefined behavior to use this value - --> $DIR/const_refers_to_static_cross_crate.rs:12:1 + --> $DIR/const_refers_to_static_cross_crate.rs:11:1 | LL | / const SLICE_MUT: &[u8; 1] = { LL | | @@ -18,21 +18,21 @@ LL | | }; = 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. error: could not evaluate constant pattern - --> $DIR/const_refers_to_static_cross_crate.rs:28:9 + --> $DIR/const_refers_to_static_cross_crate.rs:43:9 | LL | SLICE_MUT => true, | ^^^^^^^^^ warning: skipping const checks - --> $DIR/const_refers_to_static_cross_crate.rs:22:15 + --> $DIR/const_refers_to_static_cross_crate.rs:21:15 | LL | unsafe { &static_cross_crate::ZERO[0] } | ^^^^^^^^^^^^^^^^^^^^^^^^ error[E0080]: it is undefined behavior to use this value - --> $DIR/const_refers_to_static_cross_crate.rs:19:1 + --> $DIR/const_refers_to_static_cross_crate.rs:18:1 | -LL | / const SLICE_MUT2: &u8 = { +LL | / const U8_MUT: &u8 = { LL | | LL | | LL | | unsafe { &static_cross_crate::ZERO[0] } @@ -43,11 +43,71 @@ LL | | }; = 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. error: could not evaluate constant pattern - --> $DIR/const_refers_to_static_cross_crate.rs:36:9 + --> $DIR/const_refers_to_static_cross_crate.rs:51:9 | -LL | SLICE_MUT2 => true, - | ^^^^^^^^^^ +LL | U8_MUT => true, + | ^^^^^^ -error: aborting due to 4 previous errors; 2 warnings emitted +warning: skipping const checks + --> $DIR/const_refers_to_static_cross_crate.rs:28:17 + | +LL | unsafe { &(*static_cross_crate::ZERO_REF)[0] } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: any use of this value will cause an error + --> $DIR/const_refers_to_static_cross_crate.rs:28:14 + | +LL | / const U8_MUT2: &u8 = { +LL | | unsafe { &(*static_cross_crate::ZERO_REF)[0] } + | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constant accesses static +LL | | +LL | | +LL | | +LL | | }; + | |__- + | +note: the lint level is defined here + --> $DIR/const_refers_to_static_cross_crate.rs:26:8 + | +LL | #[warn(const_err)] + | ^^^^^^^^^ + +error: could not evaluate constant pattern + --> $DIR/const_refers_to_static_cross_crate.rs:61:9 + | +LL | U8_MUT2 => true, + | ^^^^^^^ + +warning: skipping const checks + --> $DIR/const_refers_to_static_cross_crate.rs:35:20 + | +LL | unsafe { match static_cross_crate::OPT_ZERO { Some(ref u) => u, None => panic!() } } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: any use of this value will cause an error + --> $DIR/const_refers_to_static_cross_crate.rs:35:51 + | +LL | / const U8_MUT3: &u8 = { +LL | | unsafe { match static_cross_crate::OPT_ZERO { Some(ref u) => u, None => panic!() } } + | | ^^^^^^^^^^^ constant accesses static +LL | | +LL | | +LL | | +LL | | }; + | |__- + | +note: the lint level is defined here + --> $DIR/const_refers_to_static_cross_crate.rs:33:8 + | +LL | #[warn(const_err)] + | ^^^^^^^^^ + +error: could not evaluate constant pattern + --> $DIR/const_refers_to_static_cross_crate.rs:68:9 + | +LL | U8_MUT3 => true, + | ^^^^^^^ + +error: aborting due to 6 previous errors; 6 warnings emitted For more information about this error, try `rustc --explain E0080`. From 07772fcf6fca44217439154aa37e4854dd5aef34 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 Apr 2020 14:56:40 +0200 Subject: [PATCH 10/14] expand comment in memory.rs with extra soundness concerns --- src/librustc_mir/interpret/memory.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 0d0ed465c1cc6..8560594b583d4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -453,7 +453,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // thing here: one maps to `GlobalAlloc::Static`, this is the "lazy" ID, // and the other one is maps to `GlobalAlloc::Memory`, this is returned by // `const_eval_raw` and it is the "resolved" ID. - // The resolved ID is never used by the interpreted progrma, it is hidden. + // The resolved ID is never used by the interpreted program, it is hidden. + // This is relied upon for soundness of const-patterns; a pointer to the resolved + // ID would "sidestep" the checks that make sure consts do not point to statics! // The `GlobalAlloc::Memory` branch here is still reachable though; when a static // contains a reference to memory that was created during its evaluation (i.e., not // to another static), those inner references only exist in "resolved" form. From 827d6f6c3d4ea2b9c7dd64b76c05c2ca204f6b76 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Wed, 29 Apr 2020 15:49:48 +0200 Subject: [PATCH 11/14] document stable counterparts of intrinsics --- src/libcore/intrinsics.rs | 96 ++++++++++++++++++++++++++++++++++----- src/libcore/panic.rs | 2 + 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 45633dc3ca51f..962bcbe61985b 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -782,7 +782,9 @@ extern "rust-intrinsic" { /// characteristics. /// /// The `locality` argument must be a constant integer and is a temporal locality specifier - /// ranging from (0) - no locality, to (3) - extremely local keep in cache + /// ranging from (0) - no locality, to (3) - extremely local keep in cache. + /// + /// This intrinsic does not have a stable counterpart. pub fn prefetch_read_data(data: *const T, locality: i32); /// The `prefetch` intrinsic is a hint to the code generator to insert a prefetch instruction /// if supported; otherwise, it is a no-op. @@ -790,7 +792,9 @@ extern "rust-intrinsic" { /// characteristics. /// /// The `locality` argument must be a constant integer and is a temporal locality specifier - /// ranging from (0) - no locality, to (3) - extremely local keep in cache + /// ranging from (0) - no locality, to (3) - extremely local keep in cache. + /// + /// This intrinsic does not have a stable counterpart. pub fn prefetch_write_data(data: *const T, locality: i32); /// The `prefetch` intrinsic is a hint to the code generator to insert a prefetch instruction /// if supported; otherwise, it is a no-op. @@ -798,7 +802,9 @@ extern "rust-intrinsic" { /// characteristics. /// /// The `locality` argument must be a constant integer and is a temporal locality specifier - /// ranging from (0) - no locality, to (3) - extremely local keep in cache + /// ranging from (0) - no locality, to (3) - extremely local keep in cache. + /// + /// This intrinsic does not have a stable counterpart. pub fn prefetch_read_instruction(data: *const T, locality: i32); /// The `prefetch` intrinsic is a hint to the code generator to insert a prefetch instruction /// if supported; otherwise, it is a no-op. @@ -806,12 +812,13 @@ extern "rust-intrinsic" { /// characteristics. /// /// The `locality` argument must be a constant integer and is a temporal locality specifier - /// ranging from (0) - no locality, to (3) - extremely local keep in cache + /// ranging from (0) - no locality, to (3) - extremely local keep in cache. + /// + /// This intrinsic does not have a stable counterpart. pub fn prefetch_write_instruction(data: *const T, locality: i32); } extern "rust-intrinsic" { - /// An atomic fence. /// /// The stabilized version of this intrinsic is available in @@ -905,12 +912,14 @@ extern "rust-intrinsic" { /// that `rustc_peek(potentially_uninitialized)` would actually /// double-check that dataflow did indeed compute that it is /// uninitialized at that point in the control flow. + /// + /// This intrinsic should not be used outside of the compiler. pub fn rustc_peek(_: T) -> T; /// Aborts the execution of the process. /// /// The stabilized version of this intrinsic is - /// [`std::process::abort`](../../std/process/fn.abort.html) + /// [`std::process::abort`](../../std/process/fn.abort.html). pub fn abort() -> !; /// Tells LLVM that this point in the code is not reachable, enabling @@ -932,21 +941,29 @@ extern "rust-intrinsic" { /// with optimization of surrounding code and reduce performance. It should /// not be used if the invariant can be discovered by the optimizer on its /// own, or if it does not enable any significant optimizations. + /// + /// This intrinsic does not have a stable counterpart. pub fn assume(b: bool); /// Hints to the compiler that branch condition is likely to be true. /// Returns the value passed to it. /// /// Any use other than with `if` statements will probably not have an effect. + /// + /// This intrinsic does not have a stable counterpart. pub fn likely(b: bool) -> bool; /// Hints to the compiler that branch condition is likely to be false. /// Returns the value passed to it. /// /// Any use other than with `if` statements will probably not have an effect. + /// + /// This intrinsic does not have a stable counterpart. pub fn unlikely(b: bool) -> bool; /// Executes a breakpoint trap, for inspection by a debugger. + /// + /// This intrinsic does not have a stable counterpart. pub fn breakpoint(); /// The size of a type in bytes. @@ -973,6 +990,9 @@ extern "rust-intrinsic" { /// [`std::mem::align_of`](../../std/mem/fn.align_of.html). #[rustc_const_stable(feature = "const_min_align_of", since = "1.40.0")] pub fn min_align_of() -> usize; + /// The prefered alignment of a type. + /// + /// This intrinsic does not have a stable counterpart. #[rustc_const_unstable(feature = "const_pref_align_of", issue = "none")] pub fn pref_align_of() -> usize; @@ -981,6 +1001,10 @@ extern "rust-intrinsic" { /// The stabilized version of this intrinsic is /// [`std::mem::size_of_val`](../../std/mem/fn.size_of_val.html). pub fn size_of_val(_: *const T) -> usize; + /// The required alignment of the referenced value. + /// + /// The stabilized version of this intrinsic is + /// [`std::mem::align_of_val`](../../std/mem/fn.align_of_val.html). pub fn min_align_of_val(_: *const T) -> usize; /// Gets a static string slice containing the name of a type. @@ -1001,22 +1025,33 @@ extern "rust-intrinsic" { /// A guard for unsafe functions that cannot ever be executed if `T` is uninhabited: /// This will statically either panic, or do nothing. + /// + /// This intrinsic does not have a stable counterpart. pub fn assert_inhabited(); /// A guard for unsafe functions that cannot ever be executed if `T` does not permit /// zero-initialization: This will statically either panic, or do nothing. + /// + /// This intrinsic does not have a stable counterpart. pub fn assert_zero_valid(); /// A guard for unsafe functions that cannot ever be executed if `T` has invalid /// bit patterns: This will statically either panic, or do nothing. + /// + /// This intrinsic does not have a stable counterpart. pub fn assert_uninit_valid(); /// Gets a reference to a static `Location` indicating where it was called. + /// + /// Consider using [`std::panic::Location::caller`](../../std/panic/struct.Location.html#method.caller) + /// instead. #[rustc_const_unstable(feature = "const_caller_location", issue = "47809")] pub fn caller_location() -> &'static crate::panic::Location<'static>; /// Moves a value out of scope without running drop glue. - /// This exists solely for `mem::forget_unsized`; normal `forget` uses `ManuallyDrop` instead. + /// + /// This exists solely for [`mem::forget_unsized`](../../std/mem/fn.forget_unsized.html); + /// normal `forget` uses `ManuallyDrop` instead. pub fn forget(_: T); /// Reinterprets the bits of a value of one type as another type. @@ -1300,6 +1335,8 @@ extern "rust-intrinsic" { /// /// The volatile parameter is set to `true`, so it will not be optimized out /// unless size is equal to zero. + /// + /// This intrinsic does not have a stable counterpart. pub fn volatile_copy_nonoverlapping_memory(dst: *mut T, src: *const T, count: usize); /// Equivalent to the appropriate `llvm.memmove.p0i8.0i8.*` intrinsic, with /// a size of `count` * `size_of::()` and an alignment of @@ -1307,6 +1344,8 @@ extern "rust-intrinsic" { /// /// The volatile parameter is set to `true`, so it will not be optimized out /// unless size is equal to zero. + /// + /// This intrinsic does not have a stable counterpart. pub fn volatile_copy_memory(dst: *mut T, src: *const T, count: usize); /// Equivalent to the appropriate `llvm.memset.p0i8.*` intrinsic, with a /// size of `count` * `size_of::()` and an alignment of @@ -1314,6 +1353,8 @@ extern "rust-intrinsic" { /// /// The volatile parameter is set to `true`, so it will not be optimized out /// unless size is equal to zero. + /// + /// This intrinsic does not have a stable counterpart. pub fn volatile_set_memory(dst: *mut T, val: u8, count: usize); /// Performs a volatile load from the `src` pointer. @@ -1329,9 +1370,13 @@ extern "rust-intrinsic" { /// Performs a volatile load from the `src` pointer /// The pointer is not required to be aligned. + /// + /// This intrinsic does not have a stable counterpart. pub fn unaligned_volatile_load(src: *const T) -> T; /// Performs a volatile store to the `dst` pointer. /// The pointer is not required to be aligned. + /// + /// This intrinsic does not have a stable counterpart. pub fn unaligned_volatile_store(dst: *mut T, val: T); /// Returns the square root of an `f32` @@ -1539,8 +1584,12 @@ extern "rust-intrinsic" { pub fn rintf64(x: f64) -> f64; /// Returns the nearest integer to an `f32`. + /// + /// This intrinsic does not have a stable counterpart. pub fn nearbyintf32(x: f32) -> f32; /// Returns the nearest integer to an `f64`. + /// + /// This intrinsic does not have a stable counterpart. pub fn nearbyintf64(x: f64) -> f64; /// Returns the nearest integer to an `f32`. Rounds half-way cases away from zero. @@ -1556,28 +1605,39 @@ extern "rust-intrinsic" { /// Float addition that allows optimizations based on algebraic rules. /// May assume inputs are finite. + /// + /// This intrinsic does not have a stable counterpart. pub fn fadd_fast(a: T, b: T) -> T; /// Float subtraction that allows optimizations based on algebraic rules. /// May assume inputs are finite. + /// + /// This intrinsic does not have a stable counterpart. pub fn fsub_fast(a: T, b: T) -> T; /// Float multiplication that allows optimizations based on algebraic rules. /// May assume inputs are finite. + /// + /// This intrinsic does not have a stable counterpart. pub fn fmul_fast(a: T, b: T) -> T; /// Float division that allows optimizations based on algebraic rules. /// May assume inputs are finite. + /// + /// This intrinsic does not have a stable counterpart. pub fn fdiv_fast(a: T, b: T) -> T; /// Float remainder that allows optimizations based on algebraic rules. /// May assume inputs are finite. + /// + /// This intrinsic does not have a stable counterpart. pub fn frem_fast(a: T, b: T) -> T; /// Convert with LLVM’s fptoui/fptosi, which may return undef for values out of range /// () /// - /// Stabilized as `f32::to_int_unchecked` and `f64::to_int_unchecked`. + /// Stabilized as [`f32::to_int_unchecked`](../../std/primitive.f32.html#method.to_int_unchecked) + /// and [`f64::to_int_unchecked`](../../std/primitive.f64.html#method.to_int_unchecked). pub fn float_to_int_unchecked(value: Float) -> Int; /// Returns the number of bits set in an integer type `T` @@ -1623,6 +1683,8 @@ extern "rust-intrinsic" { /// Like `ctlz`, but extra-unsafe as it returns `undef` when /// given an `x` with value `0`. /// + /// This intrinsic does not have a stable counterpart. + /// /// # Examples /// /// ``` @@ -1672,6 +1734,8 @@ extern "rust-intrinsic" { /// Like `cttz`, but extra-unsafe as it returns `undef` when /// given an `x` with value `0`. /// + /// This intrinsic does not have a stable counterpart. + /// /// # Examples /// /// ``` @@ -1728,12 +1792,14 @@ extern "rust-intrinsic" { /// Performs an exact division, resulting in undefined behavior where /// `x % y != 0` or `y == 0` or `x == T::MIN && y == -1` + /// + /// This intrinsic does not have a stable counterpart. pub fn exact_div(x: T, y: T) -> T; /// Performs an unchecked division, resulting in undefined behavior /// where y = 0 or x = `T::MIN` and y = -1 /// - /// The stabilized versions of this intrinsic are available on the integer + /// Safe wrappers for this intrinsic are available on the integer /// primitives via the `checked_div` method. For example, /// [`std::u32::checked_div`](../../std/primitive.u32.html#method.checked_div) #[rustc_const_unstable(feature = "const_int_unchecked_arith", issue = "none")] @@ -1741,7 +1807,7 @@ extern "rust-intrinsic" { /// Returns the remainder of an unchecked division, resulting in /// undefined behavior where y = 0 or x = `T::MIN` and y = -1 /// - /// The stabilized versions of this intrinsic are available on the integer + /// Safe wrappers for this intrinsic are available on the integer /// primitives via the `checked_rem` method. For example, /// [`std::u32::checked_rem`](../../std/primitive.u32.html#method.checked_rem) #[rustc_const_unstable(feature = "const_int_unchecked_arith", issue = "none")] @@ -1750,7 +1816,7 @@ extern "rust-intrinsic" { /// Performs an unchecked left shift, resulting in undefined behavior when /// y < 0 or y >= N, where N is the width of T in bits. /// - /// The stabilized versions of this intrinsic are available on the integer + /// Safe wrappers for this intrinsic are available on the integer /// primitives via the `checked_shl` method. For example, /// [`std::u32::checked_shl`](../../std/primitive.u32.html#method.checked_shl) #[rustc_const_stable(feature = "const_int_unchecked", since = "1.40.0")] @@ -1758,7 +1824,7 @@ extern "rust-intrinsic" { /// Performs an unchecked right shift, resulting in undefined behavior when /// y < 0 or y >= N, where N is the width of T in bits. /// - /// The stabilized versions of this intrinsic are available on the integer + /// Safe wrappers for this intrinsic are available on the integer /// primitives via the `checked_shr` method. For example, /// [`std::u32::checked_shr`](../../std/primitive.u32.html#method.checked_shr) #[rustc_const_stable(feature = "const_int_unchecked", since = "1.40.0")] @@ -1766,16 +1832,22 @@ extern "rust-intrinsic" { /// Returns the result of an unchecked addition, resulting in /// undefined behavior when `x + y > T::MAX` or `x + y < T::MIN`. + /// + /// This intrinsic does not have a stable counterpart. #[rustc_const_unstable(feature = "const_int_unchecked_arith", issue = "none")] pub fn unchecked_add(x: T, y: T) -> T; /// Returns the result of an unchecked subtraction, resulting in /// undefined behavior when `x - y > T::MAX` or `x - y < T::MIN`. + /// + /// This intrinsic does not have a stable counterpart. #[rustc_const_unstable(feature = "const_int_unchecked_arith", issue = "none")] pub fn unchecked_sub(x: T, y: T) -> T; /// Returns the result of an unchecked multiplication, resulting in /// undefined behavior when `x * y > T::MAX` or `x * y < T::MIN`. + /// + /// This intrinsic does not have a stable counterpart. #[rustc_const_unstable(feature = "const_int_unchecked_arith", issue = "none")] pub fn unchecked_mul(x: T, y: T) -> T; diff --git a/src/libcore/panic.rs b/src/libcore/panic.rs index 575f51d0a7d56..1ffd493d8130b 100644 --- a/src/libcore/panic.rs +++ b/src/libcore/panic.rs @@ -228,6 +228,8 @@ impl<'a> Location<'a> { /// assert_ne!(this_location.line(), another_location.line()); /// assert_ne!(this_location.column(), another_location.column()); /// ``` + // FIXME: When stabilizing this method, please also update the documentation + // of `intrinsics::caller_location`. #[unstable( feature = "track_caller", reason = "uses #[track_caller] which is not yet stable", From a9858791134253d004971664fd7e9bd9b0983723 Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Wed, 29 Apr 2020 11:41:34 +0800 Subject: [PATCH 12/14] Suggest deref when coercing `ty::Ref` to `ty::RawPtr` --- src/librustc_typeck/check/coercion.rs | 6 ++-- src/librustc_typeck/check/demand.rs | 38 ++++++++++++++++++++++++- src/test/ui/issues/issue-32122-1.fixed | 17 +++++++++++ src/test/ui/issues/issue-32122-1.rs | 17 +++++++++++ src/test/ui/issues/issue-32122-1.stderr | 16 +++++++++++ src/test/ui/issues/issue-32122-2.fixed | 28 ++++++++++++++++++ src/test/ui/issues/issue-32122-2.rs | 28 ++++++++++++++++++ src/test/ui/issues/issue-32122-2.stderr | 16 +++++++++++ 8 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/issues/issue-32122-1.fixed create mode 100644 src/test/ui/issues/issue-32122-1.rs create mode 100644 src/test/ui/issues/issue-32122-1.stderr create mode 100644 src/test/ui/issues/issue-32122-2.fixed create mode 100644 src/test/ui/issues/issue-32122-2.rs create mode 100644 src/test/ui/issues/issue-32122-2.stderr diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index c1a3ff518b338..5d1a1a164855d 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -74,7 +74,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode}; use smallvec::{smallvec, SmallVec}; use std::ops::Deref; -struct Coerce<'a, 'tcx> { +pub struct Coerce<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, cause: ObligationCause<'tcx>, use_lub: bool, @@ -124,7 +124,7 @@ fn success<'tcx>( } impl<'f, 'tcx> Coerce<'f, 'tcx> { - fn new( + pub fn new( fcx: &'f FnCtxt<'f, 'tcx>, cause: ObligationCause<'tcx>, allow_two_phase: AllowTwoPhase, @@ -132,7 +132,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { Coerce { fcx, cause, allow_two_phase, use_lub: false } } - fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> { + pub fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> { self.commit_if_ok(|_| { if self.use_lub { self.at(&self.cause, self.fcx.param_env).lub(b, a) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 8c3a010303768..bfc5419cc92bf 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -1,3 +1,4 @@ +use crate::check::coercion::Coerce; use crate::check::FnCtxt; use rustc_infer::infer::InferOk; use rustc_trait_selection::infer::InferCtxtExt as _; @@ -8,8 +9,9 @@ use rustc_ast::util::parser::PREC_POSTFIX; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::{is_range_literal, Node}; +use rustc_middle::traits::ObligationCauseCode; use rustc_middle::ty::adjustment::AllowTwoPhase; -use rustc_middle::ty::{self, AssocItem, Ty}; +use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut}; use rustc_span::symbol::sym; use rustc_span::Span; @@ -539,6 +541,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return Some((sp, "consider removing the borrow", code)); } } + ( + _, + &ty::RawPtr(TypeAndMut { ty: _, mutbl: hir::Mutability::Not }), + &ty::Ref(_, _, hir::Mutability::Not), + ) => { + let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable); + // We don't ever need two-phase here since we throw out the result of the coercion + let coerce = Coerce::new(self, cause, AllowTwoPhase::No); + + if let Some(steps) = + coerce.autoderef(sp, checked_ty).skip(1).find_map(|(referent_ty, steps)| { + coerce + .unify( + coerce.tcx.mk_ptr(ty::TypeAndMut { + mutbl: hir::Mutability::Not, + ty: referent_ty, + }), + expected, + ) + .ok() + .map(|_| steps) + }) + { + // The pointer type implements `Copy` trait so the suggestion is always valid. + if let Ok(code) = sm.span_to_snippet(sp) { + if code.starts_with('&') { + let derefs = "*".repeat(steps - 1); + let message = "consider dereferencing the reference"; + let suggestion = format!("&{}{}", derefs, code[1..].to_string()); + return Some((sp, message, suggestion)); + } + } + } + } _ if sp == expr.span && !is_macro => { // Check for `Deref` implementations by constructing a predicate to // prove: `::Output == U` diff --git a/src/test/ui/issues/issue-32122-1.fixed b/src/test/ui/issues/issue-32122-1.fixed new file mode 100644 index 0000000000000..4fc5f64ff9a45 --- /dev/null +++ b/src/test/ui/issues/issue-32122-1.fixed @@ -0,0 +1,17 @@ +// run-rustfix +use std::ops::Deref; + +struct Foo(u8); + +impl Deref for Foo { + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +fn main() { + let a = Foo(0); + // Should suggest `&*` when coercing &ty to *const ty + let _: *const u8 = &*a; //~ ERROR mismatched types +} diff --git a/src/test/ui/issues/issue-32122-1.rs b/src/test/ui/issues/issue-32122-1.rs new file mode 100644 index 0000000000000..3c4859f07a2e7 --- /dev/null +++ b/src/test/ui/issues/issue-32122-1.rs @@ -0,0 +1,17 @@ +// run-rustfix +use std::ops::Deref; + +struct Foo(u8); + +impl Deref for Foo { + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +fn main() { + let a = Foo(0); + // Should suggest `&*` when coercing &ty to *const ty + let _: *const u8 = &a; //~ ERROR mismatched types +} diff --git a/src/test/ui/issues/issue-32122-1.stderr b/src/test/ui/issues/issue-32122-1.stderr new file mode 100644 index 0000000000000..313de275c53ee --- /dev/null +++ b/src/test/ui/issues/issue-32122-1.stderr @@ -0,0 +1,16 @@ +error[E0308]: mismatched types + --> $DIR/issue-32122-1.rs:16:24 + | +LL | let _: *const u8 = &a; + | --------- ^^ + | | | + | | expected `u8`, found struct `Foo` + | | help: consider dereferencing the reference: `&*a` + | expected due to this + | + = note: expected raw pointer `*const u8` + found reference `&Foo` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/issues/issue-32122-2.fixed b/src/test/ui/issues/issue-32122-2.fixed new file mode 100644 index 0000000000000..cee0e59297657 --- /dev/null +++ b/src/test/ui/issues/issue-32122-2.fixed @@ -0,0 +1,28 @@ +// run-rustfix +use std::ops::Deref; +struct Bar(u8); +struct Foo(Bar); +struct Emm(Foo); +impl Deref for Bar{ + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Emm { + type Target = Foo; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +fn main() { + let a = Emm(Foo(Bar(0))); + // Should suggest `&***` even when deref is pretty deep + let _: *const u8 = &***a; //~ ERROR mismatched types +} diff --git a/src/test/ui/issues/issue-32122-2.rs b/src/test/ui/issues/issue-32122-2.rs new file mode 100644 index 0000000000000..39e9df4224e74 --- /dev/null +++ b/src/test/ui/issues/issue-32122-2.rs @@ -0,0 +1,28 @@ +// run-rustfix +use std::ops::Deref; +struct Bar(u8); +struct Foo(Bar); +struct Emm(Foo); +impl Deref for Bar{ + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Emm { + type Target = Foo; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +fn main() { + let a = Emm(Foo(Bar(0))); + // Should suggest `&***` even when deref is pretty deep + let _: *const u8 = &a; //~ ERROR mismatched types +} diff --git a/src/test/ui/issues/issue-32122-2.stderr b/src/test/ui/issues/issue-32122-2.stderr new file mode 100644 index 0000000000000..959a49507e4f5 --- /dev/null +++ b/src/test/ui/issues/issue-32122-2.stderr @@ -0,0 +1,16 @@ +error[E0308]: mismatched types + --> $DIR/issue-32122-2.rs:27:24 + | +LL | let _: *const u8 = &a; + | --------- ^^ + | | | + | | expected `u8`, found struct `Emm` + | | help: consider dereferencing the reference: `&***a` + | expected due to this + | + = note: expected raw pointer `*const u8` + found reference `&Emm` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 4813a81432da746d9094438760bcac5c161c75fe Mon Sep 17 00:00:00 2001 From: David Freese Date: Wed, 29 Apr 2020 11:50:23 -0700 Subject: [PATCH 13/14] Add clarification on std::cfg macro docs v. #[cfg] attribute The wording was discussed, to a limited degree in #71679. This tries to address some confusion I as well as someone else had independently when looking at this macro. Fixes #71679 --- src/libcore/macros/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 18d5eaa964849..8de6be86e34ba 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -1159,6 +1159,10 @@ pub(crate) mod builtin { /// The syntax given to this macro is the same syntax as the [`cfg`] /// attribute. /// + /// `cfg!`, unlike `#[cfg]`, does not remove any code and only evaluates to true or false. For + /// example, this means all code in an if/else block needs to be valid when `cfg!` is used for + /// the condition, regardless of what `cfg!` is evaluating. + /// /// [`cfg`]: ../reference/conditional-compilation.html#the-cfg-attribute /// /// # Examples From 610f94423100b9ec6b5190032c9e213af5c099de Mon Sep 17 00:00:00 2001 From: David Freese Date: Wed, 29 Apr 2020 12:16:32 -0700 Subject: [PATCH 14/14] Update src/libcore/macros/mod.rs Co-Authored-By: kennytm --- src/libcore/macros/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 8de6be86e34ba..14bfa8bab8939 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -1160,7 +1160,7 @@ pub(crate) mod builtin { /// attribute. /// /// `cfg!`, unlike `#[cfg]`, does not remove any code and only evaluates to true or false. For - /// example, this means all code in an if/else block needs to be valid when `cfg!` is used for + /// example, all blocks in an if/else expression need to be valid when `cfg!` is used for /// the condition, regardless of what `cfg!` is evaluating. /// /// [`cfg`]: ../reference/conditional-compilation.html#the-cfg-attribute