Skip to content

Commit 26d376a

Browse files
committed
fix(engine/F*): self_ -> self for clauses of methods
Consider the following piece of code: ```rust struct S(pub u8); impl S { #[hax_lib::requires(self.0 == 0)] fn f(&self) {} } ``` The `requires` annotation produces a standalone function that looks like the following: ```rust fn requires(self_: S) -> bool { self_.0 == 0 } ``` Here, we can't use `self`: this is a reserved keyword in Rust, and we are not in a `impl` context. Thus, the macro renames `self` into `self_`. The bug described in #1276 comes from this renaming not being reverted in the engine. With this commit, when the F* backend inteprets such clauses, if the function on which a clause exists have a `self` as first argument, we now substitute the first argument of the clause function to replace it with the first argument of the original function. In the example above, we replace `self_` with `self`. This PR also improves the macro to always pick a fresh name, not always `self_`.
1 parent 2b75d75 commit 26d376a

File tree

5 files changed

+82
-8
lines changed

5 files changed

+82
-8
lines changed

engine/backends/fstar/fstar_backend.ml

+27-6
Original file line numberDiff line numberDiff line change
@@ -867,15 +867,31 @@ struct
867867
F.mk_refined binder_name (pty span ty) (fun ~x -> pexpr refinement)
868868
| None -> pty span ty
869869

870-
(* let prefined_ty span (binder : string) (ty : ty) (refinement : expr) : *)
871-
(* F.AST.term = *)
872-
(* F.mk_refined binder (pty span ty) (pexpr refinement) *)
873-
874-
let add_clauses_effect_type ~no_tot_abbrev (attrs : attrs) typ : F.AST.typ =
870+
let add_clauses_effect_type ~self ~no_tot_abbrev (attrs : attrs) typ :
871+
F.AST.typ =
875872
let attr_term ?keep_last_args ?map_expr kind f =
873+
(* A clause on a method with a `self` produces a function whose first argument is `self_`.
874+
`subst_self` will substitute that first argument `self_` into the provided local identifier `self`.
875+
*)
876+
let subst_self : (expr -> expr) option =
877+
(* If `self` was present on the original function. *)
878+
let* self = self in
879+
(* Lookup the pre/post/decreases function, get the first argument: that is `self`. *)
880+
let* self' =
881+
let* _, params, _ = Attrs.associated_fn kind attrs in
882+
let* first_param = List.hd params in
883+
let* { var; _ } = Destruct.pat_PBinding first_param.pat in
884+
Some var
885+
in
886+
let f id = if [%eq: local_ident] self' id then self else id in
887+
Some ((U.Mappers.rename_local_idents f)#visit_expr ())
888+
in
876889
Attrs.associated_expr ?keep_last_args kind attrs
877890
|> Option.map
878-
~f:(Option.value ~default:Fn.id map_expr >> pexpr >> f >> F.term)
891+
~f:
892+
(Option.value ~default:Fn.id subst_self
893+
>> Option.value ~default:Fn.id map_expr
894+
>> pexpr >> f >> F.term)
879895
in
880896
let decreases =
881897
let visitor =
@@ -1030,6 +1046,11 @@ struct
10301046
let is_const = List.is_empty params in
10311047
let ty =
10321048
add_clauses_effect_type
1049+
~self:
1050+
(let* hd = List.hd params in
1051+
let* { var; _ } = Destruct.pat_PBinding hd.pat in
1052+
let*? () = String.equal var.name "self" in
1053+
Some var)
10331054
~no_tot_abbrev:(ctx.interface_mode && not is_const)
10341055
e.attrs (pty body.span body.typ)
10351056
in

hax-lib/macros/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ paste = { version = "1.0.15" }
1919
syn = { version = "2.0", features = ["full", "visit-mut", "visit"] }
2020

2121
[dependencies]
22-
syn = { version = "2.0", features = ["full", "visit-mut"] }
22+
syn = { version = "2.0", features = ["full", "visit", "visit-mut"] }
2323
proc-macro2 = { workspace = true }
2424
quote = { workspace = true }
2525

hax-lib/macros/src/utils.rs

+31-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use syn::visit::Visit;
2+
13
use crate::prelude::*;
24
use crate::rewrite_self::*;
35

@@ -140,6 +142,29 @@ pub(crate) fn expect_future_expr(e: &Expr) -> Option<std::result::Result<Ident,
140142
None
141143
}
142144

145+
#[derive(Default)]
146+
pub struct IdentCollector {
147+
pub idents: Vec<Ident>,
148+
}
149+
150+
impl<'ast> syn::visit::Visit<'ast> for IdentCollector {
151+
fn visit_ident(&mut self, ident: &'ast Ident) {
152+
self.idents.push(ident.clone());
153+
}
154+
}
155+
156+
impl IdentCollector {
157+
/// Returns a fresh identifier with the given prefix that is not in the collected identifiers.
158+
pub fn fresh_ident(&self, prefix: &str) -> Ident {
159+
let idents: HashSet<&Ident> = HashSet::from_iter(self.idents.iter());
160+
let mk = |s| Ident::new(s, Span::call_site());
161+
std::iter::once(mk(prefix))
162+
.chain((0u64..).map(|i| Ident::new(&format!("{}{}", prefix, i), Span::call_site())))
163+
.find(|ident| !idents.contains(ident))
164+
.unwrap()
165+
}
166+
}
167+
143168
/// Rewrites `future(x)` nodes in an expression when (1) `x` is an
144169
/// ident and (2) the ident `x` is contained in the HashSet.
145170
struct RewriteFuture(HashSet<String>);
@@ -206,7 +231,12 @@ pub fn make_fn_decoration(
206231
mut generics: Option<Generics>,
207232
self_type: Option<Type>,
208233
) -> (TokenStream, AttrPayload) {
209-
let self_ident: Ident = syn::parse_quote! {self_};
234+
let self_ident: Ident = {
235+
let mut idents = IdentCollector::default();
236+
idents.visit_expr(&phi);
237+
idents.visit_signature(&signature);
238+
idents.fresh_ident("self_")
239+
};
210240
let error = {
211241
let mut rewriter = RewriteSelf::new(self_ident, self_type);
212242
rewriter.visit_expr_mut(&mut phi);

test-harness/src/snapshots/toolchain__attributes into-fstar.snap

+13
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,19 @@ class t_T (v_Self: Type0) = {
126126
f_v:x0: v_Self -> Prims.Pure v_Self (f_v_pre x0) (fun result -> f_v_post x0 result)
127127
}
128128
'''
129+
"Attributes.Issue_1276_.fst" = '''
130+
module Attributes.Issue_1276_
131+
#set-options "--fuel 0 --ifuel 1 --z3rlimit 15"
132+
open Core
133+
open FStar.Mul
134+
135+
type t_S = | S : u8 -> t_S
136+
137+
let impl_S__f (self: t_S) (self_ self_0_ self_1_ self_2_: u8)
138+
: Prims.Pure Prims.unit
139+
(requires self._0 =. mk_u8 0 && self_ =. self_1_ && self_2_ =. mk_u8 9)
140+
(fun _ -> Prims.l_True) = ()
141+
'''
129142
"Attributes.Nested_refinement_elim.fst" = '''
130143
module Attributes.Nested_refinement_elim
131144
#set-options "--fuel 0 --ifuel 1 --z3rlimit 15"

tests/attributes/src/lib.rs

+10
Original file line numberDiff line numberDiff line change
@@ -434,3 +434,13 @@ mod props {
434434
!(p | y).implies(forall(|x: u8| x <= u8::MAX) & exists(|x: u16| x > 300))
435435
}
436436
}
437+
438+
mod issue_1276 {
439+
struct S(pub u8);
440+
441+
#[hax_lib::attributes]
442+
impl S {
443+
#[hax_lib::requires(self.0 == 0 && self_ == self_1 && self_2 == 9)]
444+
fn f(&self, self_: u8, self_0: u8, self_1: u8, self_2: u8) {}
445+
}
446+
}

0 commit comments

Comments
 (0)