From 3feac59b794acf326c0efebaabd500e47fc65ba9 Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Mon, 7 Apr 2025 17:42:08 -0700 Subject: [PATCH] Fix unreachable expression warning Invert the order that we pass the arguments to the `contract_check_ensures` function to avoid the warning when the tail of the function is unreachable. Note that the call itself is also unreachable, but we have already handled that case by ignoring unreachable call for contract calls. --- compiler/rustc_ast_lowering/src/expr.rs | 3 +-- compiler/rustc_ast_lowering/src/item.rs | 9 ++++++++- .../rustc_hir_analysis/src/check/intrinsic.rs | 2 +- library/core/src/contracts.rs | 15 ++++++++++----- library/core/src/intrinsics/mod.rs | 11 +++++++++-- .../contract-attributes-nest.chk_pass.stderr | 13 +------------ tests/ui/contracts/contract-attributes-nest.rs | 1 - ...ontract-attributes-nest.unchk_fail_post.stderr | 13 +------------ ...contract-attributes-nest.unchk_fail_pre.stderr | 13 +------------ .../contract-attributes-nest.unchk_pass.stderr | 13 +------------ .../contract-captures-via-closure-noncopy.stderr | 1 + .../contract-ast-extensions-nest.rs | 1 - .../internal_machinery/contract-intrinsics.rs | 4 ++-- .../internal_machinery/contract-lang-items.rs | 2 +- .../internal_machinery/internal-feature-gating.rs | 2 +- .../internal-feature-gating.stderr | 2 +- 16 files changed, 39 insertions(+), 66 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 0b7a76304311..c3f57e111830 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -401,11 +401,10 @@ impl<'hir> LoweringContext<'_, 'hir> { cond_hir_id: HirId, ) -> &'hir hir::Expr<'hir> { let cond_fn = self.expr_ident(span, cond_ident, cond_hir_id); - let span = self.mark_span_with_reason(DesugaringKind::Contract, span, None); let call_expr = self.expr_call_lang_item_fn_mut( span, hir::LangItem::ContractCheckEnsures, - arena_vec![self; *expr, *cond_fn], + arena_vec![self; *cond_fn, *expr], ); self.arena.alloc(call_expr) } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 28f596ac0926..c89112fc5a37 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1209,8 +1209,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], ); @@ -1220,6 +1225,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); diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 290e47b42b56..e54fa89328cd 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -236,7 +236,7 @@ pub fn check_intrinsic_type( // where C: for<'a> Fn(&'a Ret) -> bool, // // so: two type params, 0 lifetime param, 0 const params, two inputs, no return - (2, 0, 0, vec![param(0), param(1)], param(0), hir::Safety::Safe) + (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 { diff --git a/library/core/src/contracts.rs b/library/core/src/contracts.rs index 829226420181..53f459debabc 100644 --- a/library/core/src/contracts.rs +++ b/library/core/src/contracts.rs @@ -2,15 +2,20 @@ 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 call helps with type inference for the predicate. +/// 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 const fn build_check_ensures(cond: C) -> C where C: Fn(&Ret) -> bool + Copy + 'static, diff --git a/library/core/src/intrinsics/mod.rs b/library/core/src/intrinsics/mod.rs index 8812cb66526b..90f686a67288 100644 --- a/library/core/src/intrinsics/mod.rs +++ b/library/core/src/intrinsics/mod.rs @@ -3453,6 +3453,10 @@ pub const fn contract_checks() -> bool { /// /// 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] @@ -3478,12 +3482,15 @@ pub const fn contract_check_requires bool + Copy>(cond: C) { /// 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 bool + Copy>(ret: Ret, cond: C) -> Ret { +pub const fn contract_check_ensures bool + Copy, Ret>(cond: C, ret: Ret) -> Ret { const_eval_select!( - @capture[Ret, C: Fn(&Ret) -> bool + Copy] { ret: Ret, cond: C } -> Ret : + @capture[C: Fn(&Ret) -> bool + Copy, Ret] { cond: C, ret: Ret } -> Ret : if const { // Do nothing ret diff --git a/tests/ui/contracts/contract-attributes-nest.chk_pass.stderr b/tests/ui/contracts/contract-attributes-nest.chk_pass.stderr index e7c42ad98a57..9ca95b8bb01a 100644 --- a/tests/ui/contracts/contract-attributes-nest.chk_pass.stderr +++ b/tests/ui/contracts/contract-attributes-nest.chk_pass.stderr @@ -7,16 +7,5 @@ LL | #![feature(contracts)] = note: see issue #128044 for more information = note: `#[warn(incomplete_features)]` on by default -warning: unreachable expression - --> $DIR/contract-attributes-nest.rs:23:1 - | -LL | #[core::contracts::ensures(|ret| *ret > 100)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable expression -... -LL | return x.baz + 50; - | ----------------- any code following this expression is unreachable - | - = note: `#[warn(unreachable_code)]` on by default - -warning: 2 warnings emitted +warning: 1 warning emitted diff --git a/tests/ui/contracts/contract-attributes-nest.rs b/tests/ui/contracts/contract-attributes-nest.rs index 7c35e54c18bc..e1e61b88f282 100644 --- a/tests/ui/contracts/contract-attributes-nest.rs +++ b/tests/ui/contracts/contract-attributes-nest.rs @@ -21,7 +21,6 @@ #[core::contracts::requires(x.baz > 0)] #[core::contracts::ensures(|ret| *ret > 100)] -//~^ WARN unreachable expression [unreachable_code] fn nest(x: Baz) -> i32 { loop { diff --git a/tests/ui/contracts/contract-attributes-nest.unchk_fail_post.stderr b/tests/ui/contracts/contract-attributes-nest.unchk_fail_post.stderr index e7c42ad98a57..9ca95b8bb01a 100644 --- a/tests/ui/contracts/contract-attributes-nest.unchk_fail_post.stderr +++ b/tests/ui/contracts/contract-attributes-nest.unchk_fail_post.stderr @@ -7,16 +7,5 @@ LL | #![feature(contracts)] = note: see issue #128044 for more information = note: `#[warn(incomplete_features)]` on by default -warning: unreachable expression - --> $DIR/contract-attributes-nest.rs:23:1 - | -LL | #[core::contracts::ensures(|ret| *ret > 100)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable expression -... -LL | return x.baz + 50; - | ----------------- any code following this expression is unreachable - | - = note: `#[warn(unreachable_code)]` on by default - -warning: 2 warnings emitted +warning: 1 warning emitted diff --git a/tests/ui/contracts/contract-attributes-nest.unchk_fail_pre.stderr b/tests/ui/contracts/contract-attributes-nest.unchk_fail_pre.stderr index e7c42ad98a57..9ca95b8bb01a 100644 --- a/tests/ui/contracts/contract-attributes-nest.unchk_fail_pre.stderr +++ b/tests/ui/contracts/contract-attributes-nest.unchk_fail_pre.stderr @@ -7,16 +7,5 @@ LL | #![feature(contracts)] = note: see issue #128044 for more information = note: `#[warn(incomplete_features)]` on by default -warning: unreachable expression - --> $DIR/contract-attributes-nest.rs:23:1 - | -LL | #[core::contracts::ensures(|ret| *ret > 100)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable expression -... -LL | return x.baz + 50; - | ----------------- any code following this expression is unreachable - | - = note: `#[warn(unreachable_code)]` on by default - -warning: 2 warnings emitted +warning: 1 warning emitted diff --git a/tests/ui/contracts/contract-attributes-nest.unchk_pass.stderr b/tests/ui/contracts/contract-attributes-nest.unchk_pass.stderr index e7c42ad98a57..9ca95b8bb01a 100644 --- a/tests/ui/contracts/contract-attributes-nest.unchk_pass.stderr +++ b/tests/ui/contracts/contract-attributes-nest.unchk_pass.stderr @@ -7,16 +7,5 @@ LL | #![feature(contracts)] = note: see issue #128044 for more information = note: `#[warn(incomplete_features)]` on by default -warning: unreachable expression - --> $DIR/contract-attributes-nest.rs:23:1 - | -LL | #[core::contracts::ensures(|ret| *ret > 100)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable expression -... -LL | return x.baz + 50; - | ----------------- any code following this expression is unreachable - | - = note: `#[warn(unreachable_code)]` on by default - -warning: 2 warnings emitted +warning: 1 warning emitted diff --git a/tests/ui/contracts/contract-captures-via-closure-noncopy.stderr b/tests/ui/contracts/contract-captures-via-closure-noncopy.stderr index 4a47671fee19..b6f2e014e0a8 100644 --- a/tests/ui/contracts/contract-captures-via-closure-noncopy.stderr +++ b/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 diff --git a/tests/ui/contracts/internal_machinery/contract-ast-extensions-nest.rs b/tests/ui/contracts/internal_machinery/contract-ast-extensions-nest.rs index 7f9c3fe28cef..6d8cd3949eed 100644 --- a/tests/ui/contracts/internal_machinery/contract-ast-extensions-nest.rs +++ b/tests/ui/contracts/internal_machinery/contract-ast-extensions-nest.rs @@ -21,7 +21,6 @@ fn nest(x: Baz) -> i32 contract_requires(|| x.baz > 0) contract_ensures(|ret| *ret > 100) - //~^ WARN unreachable expression [unreachable_code] { loop { return x.baz + 50; diff --git a/tests/ui/contracts/internal_machinery/contract-intrinsics.rs b/tests/ui/contracts/internal_machinery/contract-intrinsics.rs index f94dfbde75fb..c62b8cca75ab 100644 --- a/tests/ui/contracts/internal_machinery/contract-intrinsics.rs +++ b/tests/ui/contracts/internal_machinery/contract-intrinsics.rs @@ -28,9 +28,9 @@ fn main() { 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); } diff --git a/tests/ui/contracts/internal_machinery/contract-lang-items.rs b/tests/ui/contracts/internal_machinery/contract-lang-items.rs index 26042cf688fd..73c591945310 100644 --- a/tests/ui/contracts/internal_machinery/contract-lang-items.rs +++ b/tests/ui/contracts/internal_machinery/contract-lang-items.rs @@ -22,7 +22,7 @@ fn foo(x: Baz) -> i32 { }; let ret = x.baz + 50; - core::intrinsics::contract_check_ensures(ret, injected_checker) + core::intrinsics::contract_check_ensures(injected_checker, ret) } struct Baz { baz: i32 } diff --git a/tests/ui/contracts/internal_machinery/internal-feature-gating.rs b/tests/ui/contracts/internal_machinery/internal-feature-gating.rs index 1b76eef6780f..6e5a7a3f9500 100644 --- a/tests/ui/contracts/internal_machinery/internal-feature-gating.rs +++ b/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); diff --git a/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr b/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr index 7302694a7874..1e39bd62e245 100644 --- a/tests/ui/contracts/internal_machinery/internal-feature-gating.stderr +++ b/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 for more information