Skip to content

Commit 3fccc81

Browse files
committed
Refine foreign-trait-method extern specs before ordinary defs
A trait-impl method without its own annotation is meant to be checked against the trait method's spec: `expected_ty` obtains it via `trait_item_ty` -> `def_ty_with_args` on the trait method def. That lookup returns `None` unless the trait method's (extern) spec is already registered, so `expected_ty` silently falls back to an unconstrained template and the impl body is never checked against the spec. Because `refine_local_defs` walked `mir_keys` in a single pass, a user impl such as `impl PartialEq for A` (low DefId) was refined before the blanket `PartialEq::eq` extern spec injected by std.rs (high DefId). The impl therefore escaped the check, and a `PartialEq` impl inconsistent with structural equality (e.g. one returning the negation) was accepted as safe, letting `assert!(x == x)` verify. Refine formula functions and foreign-trait-method extern specs in a first pass so the trait method's spec is registered before ordinary trait-impl methods consult it, independent of iteration order. Restrict the hoist to foreign targets: a local trait method's `#[context]`/extern-spec companion and its plain method register the same key and rely on the companion (higher DefId) being processed last to win the overwrite, so hoisting it ahead of the plain method would drop the companion's contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VowL2RdawM4Xch4tsdbSf3
1 parent af7cd99 commit 3fccc81

3 files changed

Lines changed: 126 additions & 3 deletions

File tree

src/analyze/crate_.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use std::collections::HashSet;
44

5+
use rustc_hir::def::DefKind;
56
use rustc_hir::def_id::CRATE_DEF_ID;
67
use rustc_middle::ty::{self as mir_ty, TyCtxt};
78
use rustc_span::def_id::LocalDefId;
@@ -60,13 +61,68 @@ impl<'tcx, 'ctx> Analyzer<'tcx, 'ctx> {
6061
}
6162

6263
fn refine_local_defs(&mut self) {
63-
for local_def_id in self.tcx.mir_keys(()) {
64-
if self.tcx.def_kind(*local_def_id).is_fn_like() {
65-
self.refine_fn_def(*local_def_id);
64+
let keys: Vec<LocalDefId> = self
65+
.tcx
66+
.mir_keys(())
67+
.iter()
68+
.copied()
69+
.filter(|def_id| self.tcx.def_kind(*def_id).is_fn_like())
70+
.collect();
71+
72+
// Refine formula functions and foreign-trait-method extern specs before the rest.
73+
//
74+
// A trait-impl method without its own annotation is checked against the trait method's
75+
// spec, which `expected_ty` obtains via `trait_item_ty` -> `def_ty_with_args` on the trait
76+
// method def. That lookup returns `None` unless the trait method's (extern) spec is already
77+
// registered, in which case the impl silently falls back to an unconstrained template and
78+
// its body is never checked against the spec. Registering foreign-trait-method extern specs
79+
// (and the formula functions their contracts depend on) first makes the lookup succeed
80+
// regardless of `mir_keys` iteration order.
81+
let prioritized: Vec<bool> = keys
82+
.iter()
83+
.map(|def_id| self.is_formula_fn_or_foreign_trait_method_extern_spec(*def_id))
84+
.collect();
85+
86+
for (&local_def_id, &prioritized) in keys.iter().zip(&prioritized) {
87+
if prioritized {
88+
self.refine_fn_def(local_def_id);
89+
}
90+
}
91+
for (&local_def_id, &prioritized) in keys.iter().zip(&prioritized) {
92+
if !prioritized {
93+
self.refine_fn_def(local_def_id);
6694
}
6795
}
6896
}
6997

98+
/// Whether `refine_local_defs` should register this def in its first pass: formula functions
99+
/// (whose contracts other defs' specs depend on) and extern specs targeting a *foreign* trait
100+
/// method (which local trait-impl methods resolve through `trait_item_ty`).
101+
fn is_formula_fn_or_foreign_trait_method_extern_spec(
102+
&mut self,
103+
local_def_id: LocalDefId,
104+
) -> bool {
105+
let tcx = self.tcx;
106+
let analyzer = self.ctx.local_def_analyzer(local_def_id);
107+
if analyzer.is_annotated_as_formula_fn() {
108+
return true;
109+
}
110+
if !analyzer.is_annotated_as_extern_spec_fn() {
111+
return false;
112+
}
113+
let target_def_id = analyzer.extern_spec_fn_target_def_id();
114+
// Restrict to a *foreign* trait method (an associated function whose parent is the trait,
115+
// e.g. the blanket `core::cmp::PartialEq::eq` spec injected by std.rs). A foreign target is
116+
// registered solely by this extern spec, so registering it early is safe and lets local
117+
// trait-impl methods find it via `trait_item_ty`. A *local* trait method must not be moved:
118+
// its `#[context]`/extern-spec companion and the plain trait method register the same key,
119+
// and the design relies on the companion (higher DefId) being processed last so it wins the
120+
// overwrite; hoisting it ahead of the plain method would drop the companion's contract.
121+
!target_def_id.is_local()
122+
&& matches!(tcx.def_kind(target_def_id), DefKind::AssocFn)
123+
&& matches!(tcx.def_kind(tcx.parent(target_def_id)), DefKind::Trait)
124+
}
125+
70126
#[tracing::instrument(skip(self), fields(def_id = %self.tcx.def_path_str(local_def_id)))]
71127
fn refine_fn_def(&mut self, local_def_id: LocalDefId) {
72128
let sig = self.ctx.fn_sig(local_def_id.to_def_id());
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//@error-in-other-file: Unsat
2+
//@compile-flags: -C debug-assertions=off
3+
4+
// A hand-written `PartialEq` impl whose result disagrees with structural equality must be
5+
// checked against the trait method's spec (`result == (*self == *other)`). Here `eq` returns the
6+
// negation, so `x == x` is `false` and the assertion can fail.
7+
8+
enum X {
9+
A(i32),
10+
}
11+
12+
impl thrust_models::Model for X {
13+
type Ty = Self;
14+
}
15+
16+
impl PartialEq for X {
17+
fn eq(&self, other: &X) -> bool {
18+
match (self, other) {
19+
(Self::A(lhs), Self::A(rhs)) => !lhs.eq(rhs),
20+
}
21+
}
22+
}
23+
24+
#[thrust::trusted]
25+
#[thrust_macros::requires(true)]
26+
#[thrust_macros::ensures(true)]
27+
fn rand() -> X {
28+
unimplemented!()
29+
}
30+
31+
fn main() {
32+
let x = rand();
33+
assert!(x == x);
34+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//@check-pass
2+
//@compile-flags: -C debug-assertions=off
3+
4+
// A hand-written `PartialEq` impl consistent with structural equality is checked against the
5+
// trait method's spec and accepted, so `x == x` holds.
6+
7+
enum X {
8+
A(i32),
9+
}
10+
11+
impl thrust_models::Model for X {
12+
type Ty = Self;
13+
}
14+
15+
impl PartialEq for X {
16+
fn eq(&self, other: &X) -> bool {
17+
match (self, other) {
18+
(Self::A(lhs), Self::A(rhs)) => lhs.eq(rhs),
19+
}
20+
}
21+
}
22+
23+
#[thrust::trusted]
24+
#[thrust_macros::requires(true)]
25+
#[thrust_macros::ensures(true)]
26+
fn rand() -> X {
27+
unimplemented!()
28+
}
29+
30+
fn main() {
31+
let x = rand();
32+
assert!(x == x);
33+
}

0 commit comments

Comments
 (0)