Rollup merge of #138374 - celinval:issue-136925-const-contract, r=compiler-errors,oli-obk,RalfJung

Enable contracts for const functions

Use `const_eval_select!()` macro to enable contract checking only at runtime. The existing contract logic relies on closures, which are not supported in constant functions.

This commit also removes one level of indirection for ensures clauses since we no longer build a closure around the ensures predicate.

Resolves #136925

**Call-out:** This is still a draft PR since CI is broken due to a new warning message for unreachable code when the bottom of the function is indeed unreachable. It's not clear to me why the warning wasn't triggered before.

r? ```@compiler-errors```

Author: Zalathar

compiler/rustc_ast_lowering/src/expr.rs

@@ -399,12 +399,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
         &mut self,
         expr: &'hir hir::Expr<'hir>,
         span: Span,
-        check_ident: Ident,
-        check_hir_id: HirId,
+        cond_ident: Ident,
+        cond_hir_id: HirId,
     ) -> &'hir hir::Expr<'hir> {
-        let checker_fn = self.expr_ident(span, check_ident, check_hir_id);
-        let span = self.mark_span_with_reason(DesugaringKind::Contract, span, None);
-        self.expr_call(span, checker_fn, std::slice::from_ref(expr))
+        let cond_fn = self.expr_ident(span, cond_ident, cond_hir_id);
+        let call_expr = self.expr_call_lang_item_fn_mut(
+            span,
+            hir::LangItem::ContractCheckEnsures,
+            arena_vec![self; *cond_fn, *expr],
+        );
+        self.arena.alloc(call_expr)
     }
 
     pub(crate) fn lower_const_block(&mut self, c: &AnonConst) -> hir::ConstBlock {

compiler/rustc_ast_lowering/src/item.rs

@@ -1206,8 +1206,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 let precond = if let Some(req) = &contract.requires {
                     // Lower the precondition check intrinsic.
                     let lowered_req = this.lower_expr_mut(&req);
+                    let req_span = this.mark_span_with_reason(
+                        DesugaringKind::Contract,
+                        lowered_req.span,
+                        None,
+                    );
                     let precond = this.expr_call_lang_item_fn_mut(
-                        req.span,
+                        req_span,
                         hir::LangItem::ContractCheckRequires,
                         &*arena_vec![this; lowered_req],
                     );
@@ -1217,6 +1222,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 };
                 let (postcond, body) = if let Some(ens) = &contract.ensures {
                     let ens_span = this.lower_span(ens.span);
+                    let ens_span =
+                        this.mark_span_with_reason(DesugaringKind::Contract, ens_span, None);
                     // Set up the postcondition `let` statement.
                     let check_ident: Ident =
                         Ident::from_str_and_span("__ensures_checker", ens_span);

compiler/rustc_hir/src/lang_items.rs

@@ -442,6 +442,8 @@ language_item_table! {
     DefaultTrait3,           sym::default_trait3,      default_trait3_trait,       Target::Trait,          GenericRequirement::None;
     DefaultTrait2,           sym::default_trait2,      default_trait2_trait,       Target::Trait,          GenericRequirement::None;
     DefaultTrait1,           sym::default_trait1,      default_trait1_trait,       Target::Trait,          GenericRequirement::None;
+
+    ContractCheckEnsures,     sym::contract_check_ensures,      contract_check_ensures_fn,      Target::Fn, GenericRequirement::None;
 }
 
 /// The requirement imposed on the generics of a lang item

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -217,15 +217,11 @@ pub(crate) 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)
-        // where C: impl Fn(&'a Ret) -> bool,
+        // contract_check_ensures::<Ret, C>(Ret, C) -> Ret
+        // where C: for<'a> Fn(&'a Ret) -> bool,
         //
-        // 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));
-        (2, 1, 0, vec![ref_ret, param(2)], tcx.types.unit, hir::Safety::Safe)
+        // so: two type params, 0 lifetime param, 0 const params, two inputs, no return
+        (2, 0, 0, vec![param(0), param(1)], param(1), hir::Safety::Safe)
     } else {
         let safety = intrinsic_operation_unsafety(tcx, intrinsic_id);
         let (n_tps, n_cts, inputs, output) = match intrinsic_name {

library/core/src/contracts.rs

@@ -2,19 +2,23 @@
 
 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).
+/// This is an identity function used as part of the desugaring of the `#[ensures]` attribute.
+///
+/// This is an existing hack to allow users to omit the type of the return value in their ensures
+/// attribute.
+///
+/// Ideally, rustc should be able to generate the type annotation.
+/// The existing lowering logic makes it rather hard to add the explicit type annotation,
+/// while the function call is fairly straight forward.
 #[unstable(feature = "contracts_internals", issue = "128044" /* compiler-team#759 */)]
+// Similar to `contract_check_requires`, we need to use the user-facing
+// `contracts` feature rather than the perma-unstable `contracts_internals`.
+// Const-checking doesn't honor allow_internal_unstable logic used by contract expansion.
+#[rustc_const_unstable(feature = "contracts", issue = "128044")]
 #[lang = "contract_build_check_ensures"]
-#[track_caller]
-pub fn build_check_ensures<Ret, C>(cond: C) -> impl (Fn(Ret) -> Ret) + Copy
+pub const fn build_check_ensures<Ret, C>(cond: C) -> C
 where
-    C: for<'a> Fn(&'a Ret) -> bool + Copy + 'static,
+    C: Fn(&Ret) -> bool + Copy + 'static,
 {
-    #[track_caller]
-    move |ret| {
-        crate::intrinsics::contract_check_ensures(&ret, cond);
-        ret
-    }
+    cond
 }

library/core/src/intrinsics/mod.rs

@@ -3402,20 +3402,62 @@ pub const fn contract_checks() -> bool {
 ///
 /// By default, if `contract_checks` is enabled, this will panic with no unwind if the condition
 /// returns false.
-#[unstable(feature = "contracts_internals", issue = "128044" /* compiler-team#759 */)]
+///
+/// Note that this function is a no-op during constant evaluation.
+#[unstable(feature = "contracts_internals", issue = "128044")]
+// Calls to this function get inserted by an AST expansion pass, which uses the equivalent of
+// `#[allow_internal_unstable]` to allow using `contracts_internals` functions. Const-checking
+// doesn't honor `#[allow_internal_unstable]`, so for the const feature gate we use the user-facing
+// `contracts` feature rather than the perma-unstable `contracts_internals`
+#[rustc_const_unstable(feature = "contracts", issue = "128044")]
 #[lang = "contract_check_requires"]
 #[rustc_intrinsic]
-pub fn contract_check_requires<C: Fn() -> 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");
-    }
+pub const fn contract_check_requires<C: Fn() -> bool + Copy>(cond: C) {
+    const_eval_select!(
+        @capture[C: Fn() -> bool + Copy] { cond: C } :
+        if const {
+                // Do nothing
+        } else {
+            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.
+///
+/// Note that this function is a no-op during constant evaluation.
+#[cfg(not(bootstrap))]
+#[unstable(feature = "contracts_internals", issue = "128044")]
+// Similar to `contract_check_requires`, we need to use the user-facing
+// `contracts` feature rather than the perma-unstable `contracts_internals`.
+// Const-checking doesn't honor allow_internal_unstable logic used by contract expansion.
+#[rustc_const_unstable(feature = "contracts", issue = "128044")]
+#[lang = "contract_check_ensures"]
+#[rustc_intrinsic]
+pub const fn contract_check_ensures<C: Fn(&Ret) -> bool + Copy, Ret>(cond: C, ret: Ret) -> Ret {
+    const_eval_select!(
+        @capture[C: Fn(&Ret) -> bool + Copy, Ret] { cond: C, ret: Ret } -> Ret :
+        if const {
+            // Do nothing
+            ret
+        } else {
+            if contract_checks() && !cond(&ret) {
+                // Emit no unwind panic in case this was a safety requirement.
+                crate::panicking::panic_nounwind("failed ensures check");
+            }
+            ret
+        }
+    )
+}
+
+/// This is the old version of contract_check_ensures kept here for bootstrap only.
+#[cfg(bootstrap)]
 #[unstable(feature = "contracts_internals", issue = "128044" /* compiler-team#759 */)]
 #[rustc_intrinsic]
 pub fn contract_check_ensures<'a, Ret, C: Fn(&'a Ret) -> bool>(ret: &'a Ret, cond: C) {

library/core/src/lib.rs

@@ -101,7 +101,6 @@
 #![feature(bstr)]
 #![feature(bstr_internals)]
 #![feature(cfg_match)]
-#![feature(closure_track_caller)]
 #![feature(const_carrying_mul_add)]
 #![feature(const_eval_select)]
 #![feature(core_intrinsics)]

tests/ui/contracts/contract-captures-via-closure-noncopy.stderr

@@ -16,6 +16,7 @@ LL | #[core::contracts::ensures({let old = x; move |ret:&Baz| ret.baz == old.baz
    | |                                        within this `{closure@$DIR/contract-captures-via-closure-noncopy.rs:12:42: 12:57}`
    | |                                        this tail expression is of type `{closure@contract-captures-via-closure-noncopy.rs:12:42}`
    | unsatisfied trait bound
+   | required by a bound introduced by this call
    |
    = help: within `{closure@$DIR/contract-captures-via-closure-noncopy.rs:12:42: 12:57}`, the trait `std::marker::Copy` is not implemented for `Baz`
 note: required because it's used within this closure

tests/ui/contracts/contract-const-fn.all_pass.stderr

@@ -0,0 +1,11 @@
+warning: the feature `contracts` is incomplete and may not be safe to use and/or cause compiler crashes
+  --> $DIR/contract-const-fn.rs:17:12
+   |
+LL | #![feature(contracts)]
+   |            ^^^^^^^^^
+   |
+   = note: see issue #128044 <https://github.com/rust-lang/rust/issues/128044> for more information
+   = note: `#[warn(incomplete_features)]` on by default
+
+warning: 1 warning emitted
+

tests/ui/contracts/contract-const-fn.rs

@@ -0,0 +1,56 @@
+//! Check if we can annotate a constant function with contracts.
+//!
+//! The contract is only checked at runtime, and it will not fail if evaluated statically.
+//! This is an existing limitation due to the existing architecture and the lack of constant
+//! closures.
+//!
+//@ revisions: all_pass runtime_fail_pre runtime_fail_post
+//
+//@ [all_pass] run-pass
+//
+//@ [runtime_fail_pre] run-fail
+//@ [runtime_fail_post] run-fail
+//
+//@ [all_pass] compile-flags: -Zcontract-checks=yes
+//@ [runtime_fail_pre] compile-flags: -Zcontract-checks=yes
+//@ [runtime_fail_post] compile-flags: -Zcontract-checks=yes
+#![feature(contracts)]
+//~^ WARN the feature `contracts` is incomplete and may not be safe to use and/or cause compiler crashes [incomplete_features]
+
+extern crate core;
+use core::contracts::*;
+
+#[requires(x < 100)]
+const fn less_than_100(x: u8) -> u8 {
+    x
+}
+
+// This is wrong on purpose.
+#[ensures(|ret| *ret)]
+const fn always_true(b: bool) -> bool {
+    b
+}
+
+const ZERO: u8 = less_than_100(0);
+// This is no-op because the contract cannot be checked at compilation time.
+const TWO_HUNDRED: u8 = less_than_100(200);
+
+/// Example from <https://github.com/rust-lang/rust/issues/136925>.
+#[ensures(move |ret: &u32| *ret > x)]
+const fn broken_sum(x: u32, y: u32) -> u32 {
+    x + y
+}
+
+fn main() {
+    assert_eq!(ZERO, 0);
+    assert_eq!(TWO_HUNDRED, 200);
+    assert_eq!(broken_sum(0, 1), 1);
+    assert_eq!(always_true(true), true);
+
+    #[cfg(runtime_fail_post)]
+    let _ok = always_true(false);
+
+    // Runtime check should fail.
+    #[cfg(runtime_fail_pre)]
+    let _200 = less_than_100(200);
+}

tests/ui/contracts/contract-const-fn.runtime_fail_post.stderr

@@ -0,0 +1,11 @@
+warning: the feature `contracts` is incomplete and may not be safe to use and/or cause compiler crashes
+  --> $DIR/contract-const-fn.rs:17:12
+   |
+LL | #![feature(contracts)]
+   |            ^^^^^^^^^
+   |
+   = note: see issue #128044 <https://github.com/rust-lang/rust/issues/128044> for more information
+   = note: `#[warn(incomplete_features)]` on by default
+
+warning: 1 warning emitted
+

tests/ui/contracts/contract-const-fn.runtime_fail_pre.stderr

@@ -0,0 +1,11 @@
+warning: the feature `contracts` is incomplete and may not be safe to use and/or cause compiler crashes
+  --> $DIR/contract-const-fn.rs:17:12
+   |
+LL | #![feature(contracts)]
+   |            ^^^^^^^^^
+   |
+   = note: see issue #128044 <https://github.com/rust-lang/rust/issues/128044> for more information
+   = note: `#[warn(incomplete_features)]` on by default
+
+warning: 1 warning emitted
+

tests/ui/contracts/internal_machinery/contract-intrinsics.rs

@@ -26,11 +26,11 @@ fn main() {
     #[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 };
+    let doubles_to_two = { let old = 2; move |ret: &u32 | ret + ret == old };
     // Always pass
-    core::intrinsics::contract_check_ensures(&1, doubles_to_two);
+    core::intrinsics::contract_check_ensures(doubles_to_two, 1);
 
     // Fail if enabled
     #[cfg(any(default, unchk_pass, chk_fail_ensures))]
-    core::intrinsics::contract_check_ensures(&2, doubles_to_two);
+    core::intrinsics::contract_check_ensures(doubles_to_two, 2);
 }

tests/ui/contracts/internal_machinery/contract-lang-items.rs

@@ -15,14 +15,14 @@
 #![feature(contracts)] // to access core::contracts
 //~^ WARN the feature `contracts` is incomplete and may not be safe to use and/or cause compiler crashes [incomplete_features]
 #![feature(contracts_internals)] // to access check_requires lang item
-
+#![feature(core_intrinsics)]
 fn foo(x: Baz) -> i32 {
     let injected_checker = {
         core::contracts::build_check_ensures(|ret| *ret > 100)
     };
 
     let ret = x.baz + 50;
-    injected_checker(ret)
+    core::intrinsics::contract_check_ensures(injected_checker, ret)
 }
 
 struct Baz { baz: i32 }

tests/ui/contracts/internal_machinery/internal-feature-gating.rs

@@ -6,7 +6,7 @@ fn main() {
     //~^ ERROR use of unstable library feature `contracts_internals`
     core::intrinsics::contract_check_requires(|| true);
     //~^ ERROR use of unstable library feature `contracts_internals`
-    core::intrinsics::contract_check_ensures(&1, |_|true);
+    core::intrinsics::contract_check_ensures( |_|true, &1);
     //~^ ERROR use of unstable library feature `contracts_internals`
 
     core::contracts::build_check_ensures(|_: &()| true);

tests/ui/contracts/internal_machinery/internal-feature-gating.stderr

@@ -41,7 +41,7 @@ LL |     core::intrinsics::contract_check_requires(|| true);
 error[E0658]: use of unstable library feature `contracts_internals`
   --> $DIR/internal-feature-gating.rs:9:5
    |
-LL |     core::intrinsics::contract_check_ensures(&1, |_|true);
+LL |     core::intrinsics::contract_check_ensures( |_|true, &1);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #128044 <https://github.com/rust-lang/rust/issues/128044> for more information