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

Move trait methods in cyclic dependencies bundling. #1075

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions engine/backends/fstar/fstar_backend.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,9 @@ let apply_phases (bo : BackendOptions.t) (items : Ast.Rust.item list) :
(* else *)
items
in
(* This is a hack that should be removed
(see https://github.com/hacspec/hax/issues/1078) *)
Dependencies.includes_for_bundled_trait_methods := true;
let items =
TransformToInputLanguage.ditems items
|> List.map ~f:unsize_as_identity
Expand Down
31 changes: 26 additions & 5 deletions engine/lib/dependencies.ml
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
open! Prelude

(** This is a hack that should be removed
(see https://github.com/hacspec/hax/issues/1078) *)
let includes_for_bundled_trait_methods = ref false

module Make (F : Features.T) = struct
module AST = Ast.Make (F)
module U = Ast_utils.Make (F)
Expand Down Expand Up @@ -453,16 +457,33 @@ module Make (F : Features.T) = struct
( name,
Concrete_ident.Create.move_under ~new_parent:new_name name
)))
| Some { v = Trait { items; _ }; _ }
when !includes_for_bundled_trait_methods ->
List.map items ~f:(fun { ti_ident; _ } ->
( ti_ident,
Concrete_ident.Create.move_under ~new_parent:new_name ti_ident
))
| _ -> []
in
let variant_and_constructors_renamings =
List.concat_map ~f:variants_renamings renamings
|> List.concat_map ~f:(fun (old_name, new_name) ->
[
(old_name, new_name);
( Concrete_ident.Create.constructor old_name,
Concrete_ident.Create.constructor new_name );
])
let trait_methods_renamings =
match from_ident old_name with
| Some { v = Trait { items; _ }; _ }
when not !includes_for_bundled_trait_methods ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was expecting to see a condition based on includes_for_bundled_trait_methods only at the place where you create Alias. I don't get why there are two.
Unrelated to this PR, I think the arguments to shallow_copy are getting unclear. I wonder if we should not inline this function: the argument variants_renamings looks unrelated to the notion of "making a shallow copy".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, maybe we should refactor. The thing is where we create aliases, we also do the renaming. But in case we don't create aliases we still want to do the renaming (but we don't want to do it twice in the other case). So that's why we have this condition here (but maybe we could improve).

List.map items ~f:(fun { ti_ident; _ } ->
( ti_ident,
Concrete_ident.Create.move_under ~new_parent:new_name
ti_ident ))
| _ -> []
in
trait_methods_renamings
@ [
(old_name, new_name);
( Concrete_ident.Create.constructor old_name,
Concrete_ident.Create.constructor new_name );
])
in
let renamings =
Map.of_alist_exn
Expand Down
4 changes: 4 additions & 0 deletions engine/lib/dependencies.mli
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ module Make (F : Features.T) : sig
val filter_by_inclusion_clauses :
Types.inclusion_clause list -> AST.item list -> AST.item list
end

val includes_for_bundled_trait_methods : bool ref
(** This is a hack that should be removed
(see https://github.com/hacspec/hax/issues/1078) *)
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,56 @@ include Cyclic_modules.M2.Rec_bundle_489499412 {d as d}

include Cyclic_modules.M2.Rec_bundle_489499412 {b as b}
'''
"Cyclic_modules.Moved_trait.Nested.fst" = '''
module Cyclic_modules.Moved_trait.Nested
#set-options "--fuel 0 --ifuel 1 --z3rlimit 15"
open Core
open FStar.Mul

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {t_St as t_St}

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {g as g}
'''
"Cyclic_modules.Moved_trait.Rec_bundle_963167032.fst" = '''
module Cyclic_modules.Moved_trait.Rec_bundle_963167032
#set-options "--fuel 0 --ifuel 1 --z3rlimit 15"
open Core
open FStar.Mul

class v_Tr (v_Self: Type0) = {
f_f_pre:v_Self -> Type0;
f_f_post:v_Self -> Prims.unit -> Type0;
f_f:x0: v_Self -> Prims.Pure Prims.unit (f_f_pre x0) (fun result -> f_f_post x0 result)
}

type t_St = | St : t_St

[@@ FStar.Tactics.Typeclasses.tcinstance]
let impl: v_Tr t_St =
{
f_f_pre = (fun (self: t_St) -> true);
f_f_post = (fun (self: t_St) (out: Prims.unit) -> true);
f_f = fun (self: t_St) -> ()
}

let g (x: t_St) : Prims.unit = f_f #t_St #FStar.Tactics.Typeclasses.solve x
'''
"Cyclic_modules.Moved_trait.fst" = '''
module Cyclic_modules.Moved_trait
#set-options "--fuel 0 --ifuel 1 --z3rlimit 15"
open Core
open FStar.Mul

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {v_Tr as v_Tr}

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {f_f_pre as f_f_pre}

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {f_f_post as f_f_post}

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {f_f as f_f}

include Cyclic_modules.Moved_trait.Rec_bundle_963167032 {impl as impl}
'''
"Cyclic_modules.Rec.fst" = '''
module Cyclic_modules.Rec
#set-options "--fuel 0 --ifuel 1 --z3rlimit 15"
Expand Down
17 changes: 17 additions & 0 deletions tests/cyclic-modules/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,20 @@ pub mod variant_constructor_b {
super::variant_constructor_a::Context::A(1)
}
}

pub mod moved_trait {
impl Tr for nested::St {
fn f(self) {}
}
pub trait Tr {
fn f(self);
}

pub mod nested {
use super::Tr;
pub struct St {}
fn g(x: St) {
x.f()
}
}
}
Loading