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

Let charon manage Self clauses #413

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nadrieril
Copy link
Member

Companion PR to AeneasVerif/charon#514.

This removes the need for managing the special Self clause in methods ourselves. One consequence is that in AeneasVerif/charon#514 I had to add a pass that removes unused Self clauses otherwise any trait impl that reuses an associated constant default would end up self-recursive. That's why you can see in the tests diff that some provided methods end up losing their Self clause: it's because of that same pass. This may be leaving the translation in a bit of an incorrect state while AeneasVerif/charon#180 isn't finished.

@Nadrieril
Copy link
Member Author

Ah, this in fact doesn't pass typechecking in lean. I wasn't sure these methods were used. I guess I have to finish AeneasVerif/charon#180 first.

@Nadrieril Nadrieril changed the title Update charon Let charon manage Self clauses Jan 9, 2025
@Nadrieril Nadrieril force-pushed the update-charon branch 2 times, most recently from 909fc15 to 722eaf0 Compare January 9, 2025 10:17
@sonmarcho
Copy link
Member

Ah, this in fact doesn't pass typechecking in lean. I wasn't sure these methods were used. I guess I have to finish AeneasVerif/charon#180 first.

What doesn't typecheck? I don't see any issue in the diffs or in CI.

@Nadrieril
Copy link
Member Author

What doesn't typecheck? I don't see any issue in the diffs or in CI.

That's because I just fixed it :) I had messed up the self clauses and didn't think I could easily fix it but I found a way.

@sonmarcho
Copy link
Member

sonmarcho commented Jan 9, 2025

Also, about the code you generate: at some point I thought of making the default methods take as inputs exactly the list of functions they need rather than a whole instance of the trait. Removing the input self clause when it is not needed is already a nice improvement (and I'm thinking that taking a list of functions may be a bit too fine-grained).

@Nadrieril
Copy link
Member Author

at some point I thought of making the default methods take as inputs exactly the list of functions they need

Yes we discussed this together :p

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