Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some fixes to model printing #256

Merged
merged 4 commits into from
Dec 2, 2024
Merged

some fixes to model printing #256

merged 4 commits into from
Dec 2, 2024

Conversation

zapashcanon
Copy link
Collaborator

this is breaking a lot of tests but this is because they seem to be calling Value.pp directly instead of calling something more high-level (like Model.pp I would say ?). Let me know what you think. :)

@filipeom
Copy link
Member

filipeom commented Dec 2, 2024

this is breaking a lot of tests but this is because they seem to be calling Value.pp directly instead of calling something more high-level (like Model.pp I would say ?). Let me know what you think. :)

Ah thanks! I forgot to do the pp_no_type 😞 I should have tested the printing of the scfg models. Anyway, I think for the failing tests we can just promote.

I would only suggest the follwing patch:

diff --git a/src/solvers/model.ml b/src/solvers/model.ml
index 6a7f858..c783718 100644
--- a/src/solvers/model.ml
+++ b/src/solvers/model.ml
@@ -23,10 +23,10 @@ let pp_bindings fmt ?(no_values = false) model =
   Fmt.list
     ~sep:(fun fmt () -> Fmt.pf fmt "@\n")
     (fun fmt (key, data) ->
-      if not no_values then Fmt.pf fmt "(%a %a)" Symbol.pp key Value.pp data
-      else
-        let t = Symbol.type_of key in
-        Fmt.pf fmt "(%a %a)" Symbol.pp key Ty.pp t )
+      let t = Symbol.type_of key in
+      if not no_values then
+        Fmt.pf fmt "(%a %a %a)" Symbol.pp key Ty.pp t Value.pp data
+      else Fmt.pf fmt "(%a %a)" Symbol.pp key Ty.pp t )
     fmt (get_bindings model)

 let pp ?(no_values = false) fmt model =

@zapashcanon
Copy link
Collaborator Author

Thanks, I applied your patch. I'm getting some failures on the tests because I'm missing some pinned stuff. Do you think you could run+promote them for me ? My hard drive is completely full and I can't install more stuff right now... 😅

@filipeom
Copy link
Member

filipeom commented Dec 2, 2024

Thanks, I applied your patch. I'm getting some failures on the tests because I'm missing some pinned stuff. Do you think you could run+promote them for me ? My hard drive is completely full and I can't install more stuff right now... 😅

Ah yes, sorry! I can finish it from here. Thank you very much! 😃

@filipeom filipeom enabled auto-merge (rebase) December 2, 2024 18:37
@filipeom filipeom merged commit 4e85919 into formalsec:main Dec 2, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants