From c2f671c150f0c52c1d0e9987241b74bb65beaf68 Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Fri, 17 Jan 2025 14:49:10 -0800 Subject: [PATCH] Improve contracts intrisics and remove wrapper function 1. Document the new intrinsics. 2. Make the intrinsics actually check the contract if enabled, and remove `contract::check_requires` function. 3. Use panic with no unwind in case contract is using to check for safety, we probably don't want to unwind. Following the same reasoning as UB checks. --- .../rustc_hir_analysis/src/check/intrinsic.rs | 10 ++--- library/core/src/contracts.rs | 27 +++----------- library/core/src/intrinsics/mod.rs | 22 +++++++++-- .../internal_machinery/contract-intrinsics.rs | 37 +++++++++++++------ .../internal_machinery/contract-lang-items.rs | 12 +----- .../internal-feature-gating.rs | 3 -- .../internal-feature-gating.stderr | 18 ++------- 7 files changed, 57 insertions(+), 72 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index c80e6d58252ff..0ca7459345c2b 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -222,17 +222,15 @@ pub fn check_intrinsic_type( }; (n_tps, 0, 0, inputs, output, hir::Safety::Unsafe) } else if intrinsic_name == sym::contract_check_ensures { - // contract_check_ensures::<'a, Ret, C>(&'a Ret, C) -> bool + // contract_check_ensures::<'a, Ret, C>(&'a Ret, C) // where C: impl Fn(&'a Ret) -> bool, // - // so: two type params, one lifetime param, 0 const params, two inputs, returns boolean + // so: two type params, one lifetime param, 0 const params, two inputs, no return let p = generics.param_at(0, tcx); let r = ty::Region::new_early_param(tcx, p.to_early_bound_region_data()); let ref_ret = Ty::new_imm_ref(tcx, r, param(1)); - // let br = ty::BoundRegion { var: ty::BoundVar::ZERO, kind: ty::BrAnon }; - // let ref_ret = Ty::new_imm_ref(tcx, ty::Region::new_bound(tcx, ty::INNERMOST, br), param(0)); - (2, 1, 0, vec![ref_ret, param(2)], tcx.types.bool, hir::Safety::Safe) + (2, 1, 0, vec![ref_ret, param(2)], tcx.types.unit, hir::Safety::Safe) } else { let safety = intrinsic_operation_unsafety(tcx, intrinsic_id); let (n_tps, n_cts, inputs, output) = match intrinsic_name { @@ -627,7 +625,7 @@ pub fn check_intrinsic_type( // contract_checks() -> bool sym::contract_checks => (0, 0, Vec::new(), tcx.types.bool), // contract_check_requires::(C) -> bool, where C: impl Fn() -> bool - sym::contract_check_requires => (1, 0, vec![param(0)], tcx.types.bool), + sym::contract_check_requires => (1, 0, vec![param(0)], tcx.types.unit), sym::simd_eq | sym::simd_ne diff --git a/library/core/src/contracts.rs b/library/core/src/contracts.rs index b155dbc213ee8..0668cacb92c60 100644 --- a/library/core/src/contracts.rs +++ b/library/core/src/contracts.rs @@ -1,38 +1,21 @@ //! Unstable module containing the unstable contracts lang items and attribute macros. +#![cfg(not(bootstrap))] -#[cfg(not(bootstrap))] -pub use crate::macros::builtin::contracts_ensures as ensures; -#[cfg(not(bootstrap))] -pub use crate::macros::builtin::contracts_requires as requires; - -/// Emitted by rustc as a desugaring of `#[requires(PRED)] fn foo(x: X) { ... }` -/// into: `fn foo(x: X) { check_requires(|| PRED) ... }` -#[cfg(not(bootstrap))] -#[unstable(feature = "rustc_contracts_internals", issue = "133866" /* compiler-team#759 */)] -#[lang = "contract_check_requires"] -#[track_caller] -pub fn check_requires bool>(c: C) { - if core::intrinsics::contract_checks() { - assert!(core::intrinsics::contract_check_requires(c), "failed requires check"); - } -} +pub use crate::macros::builtin::{contracts_ensures as ensures, contracts_requires as requires}; /// Emitted by rustc as a desugaring of `#[ensures(PRED)] fn foo() -> R { ... [return R;] ... }` /// into: `fn foo() { let _check = build_check_ensures(|ret| PRED) ... [return _check(R);] ... }` /// (including the implicit return of the tail expression, if any). -#[cfg(not(bootstrap))] #[unstable(feature = "rustc_contracts_internals", issue = "133866" /* compiler-team#759 */)] #[lang = "contract_build_check_ensures"] #[track_caller] -pub fn build_check_ensures(c: C) -> impl (FnOnce(Ret) -> Ret) + Copy +pub fn build_check_ensures(cond: C) -> impl (Fn(Ret) -> Ret) + Copy where - C: for<'a> FnOnce(&'a Ret) -> bool + Copy + 'static, + C: for<'a> Fn(&'a Ret) -> bool + Copy + 'static, { #[track_caller] move |ret| { - if core::intrinsics::contract_checks() { - assert!(core::intrinsics::contract_check_ensures(&ret, c), "failed ensures check"); - } + crate::intrinsics::contract_check_ensures(&ret, cond); ret } } diff --git a/library/core/src/intrinsics/mod.rs b/library/core/src/intrinsics/mod.rs index 10c50e24c52b4..8ff3b8daf6ced 100644 --- a/library/core/src/intrinsics/mod.rs +++ b/library/core/src/intrinsics/mod.rs @@ -4057,18 +4057,32 @@ pub const fn contract_checks() -> bool { false } +/// Check if the pre-condition `cond` has been met. +/// +/// By default, if `contract_checks` is enabled, this will panic with no unwind if the condition +/// returns false. #[cfg(not(bootstrap))] #[unstable(feature = "rustc_contracts_internals", issue = "133866" /* compiler-team#759 */)] +#[lang = "contract_check_requires"] #[rustc_intrinsic] -pub fn contract_check_requires bool>(c: C) -> bool { - c() +pub fn contract_check_requires bool>(cond: C) { + if contract_checks() && !cond() { + // Emit no unwind panic in case this was a safety requirement. + crate::panicking::panic_nounwind("failed requires check"); + } } +/// Check if the post-condition `cond` has been met. +/// +/// By default, if `contract_checks` is enabled, this will panic with no unwind if the condition +/// returns false. #[cfg(not(bootstrap))] #[unstable(feature = "rustc_contracts_internals", issue = "133866" /* compiler-team#759 */)] #[rustc_intrinsic] -pub fn contract_check_ensures<'a, Ret, C: FnOnce(&'a Ret) -> bool>(ret: &'a Ret, c: C) -> bool { - c(ret) +pub fn contract_check_ensures<'a, Ret, C: Fn(&'a Ret) -> bool>(ret: &'a Ret, cond: C) { + if contract_checks() && !cond(ret) { + crate::panicking::panic_nounwind("failed ensures check"); + } } /// The intrinsic will return the size stored in that vtable. diff --git a/tests/ui/contracts/internal_machinery/contract-intrinsics.rs b/tests/ui/contracts/internal_machinery/contract-intrinsics.rs index 2e1be01e7cabc..8c70c1a85f6d3 100644 --- a/tests/ui/contracts/internal_machinery/contract-intrinsics.rs +++ b/tests/ui/contracts/internal_machinery/contract-intrinsics.rs @@ -1,23 +1,36 @@ -//@ run-pass -//@ revisions: yes no none -//@ [yes] compile-flags: -Zcontract-checks=yes -//@ [no] compile-flags: -Zcontract-checks=no +//@ revisions: default unchk_pass chk_pass chk_fail_ensures chk_fail_requires +// +//@ [default] run-pass +//@ [unchk_pass] run-pass +//@ [chk_pass] run-pass +//@ [chk_fail_requires] run-fail +//@ [chk_fail_ensures] run-fail +// +//@ [unchk_pass] compile-flags: -Zcontract-checks=no +//@ [chk_pass] compile-flags: -Zcontract-checks=yes +//@ [chk_fail_requires] compile-flags: -Zcontract-checks=yes +//@ [chk_fail_ensures] compile-flags: -Zcontract-checks=yes #![feature(cfg_contract_checks, rustc_contracts_internals, core_intrinsics)] fn main() { - #[cfg(none)] // default: disabled + #[cfg(any(default, unchk_pass))] // default: disabled assert_eq!(core::intrinsics::contract_checks(), false); - #[cfg(yes)] // explicitly enabled + #[cfg(chk_pass)] // explicitly enabled assert_eq!(core::intrinsics::contract_checks(), true); - #[cfg(no)] // explicitly disabled - assert_eq!(core::intrinsics::contract_checks(), false); + // always pass + core::intrinsics::contract_check_requires(|| true); - assert_eq!(core::intrinsics::contract_check_requires(|| true), true); - assert_eq!(core::intrinsics::contract_check_requires(|| false), false); + // fail if enabled + #[cfg(any(default, unchk_pass, chk_fail_requires))] + core::intrinsics::contract_check_requires(|| false); let doubles_to_two = { let old = 2; move |ret| ret + ret == old }; - assert_eq!(core::intrinsics::contract_check_ensures(&1, doubles_to_two), true); - assert_eq!(core::intrinsics::contract_check_ensures(&2, doubles_to_two), false); + // Always pass + core::intrinsics::contract_check_ensures(&1, doubles_to_two); + + // Fail if enabled + #[cfg(any(default, unchk_pass, chk_fail_ensures))] + core::intrinsics::contract_check_ensures(&2, doubles_to_two); } diff --git a/tests/ui/contracts/internal_machinery/contract-lang-items.rs b/tests/ui/contracts/internal_machinery/contract-lang-items.rs index 5af467485b10e..ff569e011f206 100644 --- a/tests/ui/contracts/internal_machinery/contract-lang-items.rs +++ b/tests/ui/contracts/internal_machinery/contract-lang-items.rs @@ -1,27 +1,21 @@ -//@ revisions: unchk_pass unchk_fail_pre unchk_fail_post chk_pass chk_fail_pre chk_fail_post +//@ revisions: unchk_pass unchk_fail_post chk_pass chk_fail_post // //@ [unchk_pass] run-pass -//@ [unchk_fail_pre] run-pass //@ [unchk_fail_post] run-pass //@ [chk_pass] run-pass // -//@ [chk_fail_pre] run-fail //@ [chk_fail_post] run-fail // //@ [unchk_pass] compile-flags: -Zcontract-checks=no -//@ [unchk_fail_pre] compile-flags: -Zcontract-checks=no //@ [unchk_fail_post] compile-flags: -Zcontract-checks=no // //@ [chk_pass] compile-flags: -Zcontract-checks=yes -//@ [chk_fail_pre] compile-flags: -Zcontract-checks=yes //@ [chk_fail_post] compile-flags: -Zcontract-checks=yes #![feature(rustc_contracts)] // to access core::contracts #![feature(rustc_contracts_internals)] // to access check_requires lang item fn foo(x: Baz) -> i32 { - core::contracts::check_requires(|| x.baz > 0); - let injected_checker = { core::contracts::build_check_ensures(|ret| *ret > 100) }; @@ -36,13 +30,9 @@ struct Baz { baz: i32 } const BAZ_PASS_PRE_POST: Baz = Baz { baz: 100 }; #[cfg(any(unchk_fail_post, chk_fail_post))] const BAZ_FAIL_POST: Baz = Baz { baz: 10 }; -#[cfg(any(unchk_fail_pre, chk_fail_pre))] -const BAZ_FAIL_PRE: Baz = Baz { baz: -10 }; fn main() { assert_eq!(foo(BAZ_PASS_PRE_POST), 150); - #[cfg(any(unchk_fail_pre, chk_fail_pre))] - foo(BAZ_FAIL_PRE); #[cfg(any(unchk_fail_post, chk_fail_post))] foo(BAZ_FAIL_POST); } diff --git a/tests/ui/contracts/internal_machinery/internal-feature-gating.rs b/tests/ui/contracts/internal_machinery/internal-feature-gating.rs index 7b5f17679421f..ee9edf3ebb0c2 100644 --- a/tests/ui/contracts/internal_machinery/internal-feature-gating.rs +++ b/tests/ui/contracts/internal_machinery/internal-feature-gating.rs @@ -9,9 +9,6 @@ fn main() { core::intrinsics::contract_check_ensures(&1, |_|true); //~^ ERROR use of unstable library feature `rustc_contracts_internals` - // lang items are guarded by rustc_contracts_internals feature gate. - core::contracts::check_requires(|| true); - //~^ ERROR use of unstable library feature `rustc_contracts_internals` core::contracts::build_check_ensures(|_: &()| true); //~^ ERROR use of unstable library feature `rustc_contracts_internals` diff --git a/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr b/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr index 2acd03b9a358f..5f9263e03e85a 100644 --- a/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr +++ b/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr @@ -1,5 +1,5 @@ error[E0658]: contract internal machinery is for internal use only - --> $DIR/internal-feature-gating.rs:19:51 + --> $DIR/internal-feature-gating.rs:16:51 | LL | fn identity_1() -> i32 rustc_contract_requires(|| true) { 10 } | ^^^^^^^^^ @@ -9,7 +9,7 @@ LL | fn identity_1() -> i32 rustc_contract_requires(|| true) { 10 } = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: contract internal machinery is for internal use only - --> $DIR/internal-feature-gating.rs:21:50 + --> $DIR/internal-feature-gating.rs:18:50 | LL | fn identity_2() -> i32 rustc_contract_ensures(|_| true) { 10 } | ^^^^^^^^^^ @@ -49,17 +49,7 @@ LL | core::intrinsics::contract_check_ensures(&1, |_|true); = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: use of unstable library feature `rustc_contracts_internals` - --> $DIR/internal-feature-gating.rs:13:5 - | -LL | core::contracts::check_requires(|| true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: see issue #133866 for more information - = help: add `#![feature(rustc_contracts_internals)]` to the crate attributes to enable - = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date - -error[E0658]: use of unstable library feature `rustc_contracts_internals` - --> $DIR/internal-feature-gating.rs:15:5 + --> $DIR/internal-feature-gating.rs:12:5 | LL | core::contracts::build_check_ensures(|_: &()| true); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -68,6 +58,6 @@ LL | core::contracts::build_check_ensures(|_: &()| true); = help: add `#![feature(rustc_contracts_internals)]` to the crate attributes to enable = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date -error: aborting due to 7 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0658`.