Skip to content

Conversation

@pcarrott
Copy link
Contributor

Added a new symbolic value Exists of (Var.t * ty) list * t to bv_values. This is needed for RUXt.

Can do the same for c_values for consistency, though it is not something I need atm.

@pcarrott pcarrott requested a review from N1ark October 20, 2025 14:52
@pcarrott pcarrott requested a review from giltho as a code owner October 20, 2025 14:52
@giltho
Copy link
Contributor

giltho commented Oct 20, 2025

C_values will go away, so no need :)

Copy link
Contributor

@N1ark N1ark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition thanks :)
Sorry ton of nitpicks just to keep the codebase clean

Comment on lines +224 to +226
| Var v ->
if List.exists (fun (v', _) -> Var.equal v v') vs then ()
else f sv
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| Var v ->
if List.exists (fun (v', _) -> Var.equal v v') vs then ()
else f sv
| Var v when List.exists (fun (v', _) -> Var.equal v v') vs -> ()

Comment on lines +365 to +367
match List.find_opt (fun (v', _) -> Var.equal v v') vs with
| Some (v, _) -> v
| None -> subst_var v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
match List.find_opt (fun (v', _) -> Var.equal v v') vs with
| Some (v, _) -> v
| None -> subst_var v
if List.exists (fun (v', _) -> Var.equal v v') vs then v
else subst_var v

Comment on lines +564 to +565
let exists vs body = Exists (vs, body) <| TBool

Copy link
Contributor

@N1ark N1ark Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let exists vs body = Exists (vs, body) <| TBool
let exists vs body =
match vs with [] -> body | _ :: _ -> Exists (vs, body) <| TBool

Comment on lines +51 to +57
let eval_without_vars vs =
let eval_var v ty =
match List.find_opt (fun (v', _) -> Var.equal v v') vs with
| Some (v, ty) -> mk_var v ty
| None -> eval_var v ty
in
eval ~eval_var
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you move this into the Exists branch since it's only used there? this also avoids an intermediary function since the inputs are known

Comment on lines 88 to +90

let rec encode_value (v : Svalue.t) =
let var v = atom (Svalue.Var.to_string v) in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm being a bit nitpicky here I'll admit but it feels more cohesive to, instead of defining this inside encode_value, having

let encode_var v = atom (Svalue.Var.to_string v) 

Comment on lines +111 to +114
| Exists (vs, sv) ->
let exists l x = list [ atom "exists"; list l; x ] in
let encode_var_memo (v, ty) = list [ var v; sort_of_ty ty ] in
exists (List.map encode_var_memo vs) (encode_value_memo sv)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I avoid direct uses of atom in encodings, as it can be error prone and overall not super nice; could you move this to solvers/smt_utils?
    Also ideally use the appropriate Smtlib functions, so i think it should rather look like
    let exists qs e = app_ "exists" [ list qs; e ]
  2. encode_var_memo seems to imply there is some memoisation happening, which there isn't; can you rename it like encode_binding please :)

Comment on lines +5 to +10
let test =
let v = Svalue.Var.of_int 42 in
let ty = Svalue.t_bool in
let sv = Svalue.mk_var v ty in
let exists = Svalue.Bool.exists [ (v, ty) ] sv in
if%sat Typed.type_ exists then return 42 else return (-1)
Copy link
Contributor

@N1ark N1ark Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last thing ; add a definition for exists in Typed, so this test is well typed, rather than having to resort to Typed.type_ at the end
The signature should be something like

val exists : (Var.t * ty) list -> [< T.sbool ] t -> [> T.sbool ] t

@N1ark
Copy link
Contributor

N1ark commented Oct 21, 2025

when possible always use the most restrained form of a function -- e.g. if a List.exists is enough, use that instead of find_opt; if you have a match like | A -> if B then X else Y | _ -> Y, just do | A when B -> X | _ -> Y, etc

@N1ark N1ark added enhancement New feature or request soteria-core Issues related to the Soteria core library solver Solver related issues and PRs: new solvers, changes to encoding, etc. labels Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request solver Solver related issues and PRs: new solvers, changes to encoding, etc. soteria-core Issues related to the Soteria core library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants