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

Support for trait clause bounds #11

Merged
merged 69 commits into from
Jun 3, 2024
Merged

Support for trait clause bounds #11

merged 69 commits into from
Jun 3, 2024

Conversation

msprotz
Copy link
Contributor

@msprotz msprotz commented May 15, 2024

This isn't quite working yet but I want to get this in place to start working through the code with @W95Psp.

This is now on-par with what used to work before.

Let's try to block some time to go over this once you've had a chance to browse the list of changes. I've tried to add comments whenever I could, so hopefully some of this makes sense? Thanks!!

protz added 5 commits May 10, 2024 14:46
Rather than having a hidden invariant, the translation from LLBC to
krml's internal AST now repeats the const generic binders within
expression binders too -- this aligns with what krml does.

This change also introduces a new kind of binder, which will act as a
stand-in for additional function parameters intended to hold trait
clause methods.
@msprotz msprotz requested a review from W95Psp May 15, 2024 03:09
@msprotz msprotz marked this pull request as ready for review May 16, 2024 15:04
@@ -172,6 +213,25 @@ module RustNames = struct
let from =
parse_pattern "core::convert::From<@T, @U>::from"

let into_u16 =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@W95Psp I heard from @sonmarcho that there is a better, more concise way to do this, so this would be another interesting mini-task to add onto this PR -- refactor this bit to have more effective matching logic here, and in the main expression translation function

Copy link
Member

Choose a reason for hiding this comment

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

What I do is actually quite simple, see this and this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to recover the values for which @t and @U matched? I guess that's what I had in mind when I wrote "a better way"

lib/AstOfLlbc.ml Show resolved Hide resolved
@@ -172,6 +213,25 @@ module RustNames = struct
let from =
parse_pattern "core::convert::From<@T, @U>::from"

let into_u16 =
Copy link
Member

Choose a reason for hiding this comment

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

What I do is actually quite simple, see this and this

lib/AstOfLlbc.ml Outdated Show resolved Hide resolved
lib/AstOfLlbc.ml Outdated Show resolved Hide resolved
lib/AstOfLlbc.ml Show resolved Hide resolved
lib/AstOfLlbc.ml Show resolved Hide resolved
lib/AstOfLlbc.ml Outdated Show resolved Hide resolved
@@ -868,16 +1181,30 @@ let rec expression_of_raw_statement (env: env) (ret_var: C.var_id) (s: C.raw_sta
Krml.Helpers.with_unit K.(EAssign (dest, maybe_addrof env ty (with_type t (EBufRead (e1, e2)))))

| Call { func = FnOpRegular fn_ptr; args; dest; _ }
when false && Charon.NameMatcher.match_fn_ptr env.name_ctx RustNames.config RustNames.from fn_ptr ->
when (
Charon.NameMatcher.match_fn_ptr env.name_ctx RustNames.config RustNames.from_u16 fn_ptr ||
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #20 as a followup

else if matches RustNames.from_i16 then Int16
else if matches RustNames.from_i32 then Int32
else if matches RustNames.from_i64 then Int64
if matches RustNames.from_u16 || matches RustNames.into_u16 then UInt16
Copy link
Member

Choose a reason for hiding this comment

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

Same remark as above

test/where_clauses_simple/src/main.rs Outdated Show resolved Hide resolved
lib/AstOfLlbc.ml Outdated Show resolved Hide resolved
msprotz and others added 25 commits May 28, 2024 11:28
…mensional arrays on the stack, and as top-level constants. This fixes #15
@msprotz msprotz merged commit c8c1e34 into main Jun 3, 2024
2 checks passed
@msprotz msprotz deleted the protz_trait_clauses branch June 3, 2024 23:23
@msprotz msprotz restored the protz_trait_clauses branch June 3, 2024 23:23
@msprotz
Copy link
Contributor Author

msprotz commented Jun 4, 2024

I'm merging this and opened #20 to track the specific issue of improving the name matcher logic.

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.

5 participants