Skip to content

fix Core.Clone #1552

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

Merged
merged 4 commits into from
Jul 24, 2025
Merged

fix Core.Clone #1552

merged 4 commits into from
Jul 24, 2025

Conversation

Parrot7483
Copy link
Collaborator

No description provided.

@Parrot7483 Parrot7483 requested a review from a team as a code owner July 11, 2025 15:05
@Parrot7483 Parrot7483 requested a review from W95Psp July 11, 2025 15:05
Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Edit: I pushed a minor change, fixing the template code for Clone produced by the F* backend in the engine.

@Parrot7483 Parrot7483 added this pull request to the merge queue Jul 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 21, 2025
@Parrot7483
Copy link
Collaborator Author

Can this be merged?

@W95Psp W95Psp enabled auto-merge July 21, 2025 14:42
@W95Psp
Copy link
Collaborator

W95Psp commented Jul 21, 2025

The rustc coverage tests had to be updated, I pushed the update

@W95Psp W95Psp added this pull request to the merge queue Jul 21, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jul 21, 2025
@W95Psp W95Psp removed this pull request from the merge queue due to a manual request Jul 21, 2025
@W95Psp
Copy link
Collaborator

W95Psp commented Jul 21, 2025

Waiting for #1558

@Parrot7483 Parrot7483 added this pull request to the merge queue Jul 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 21, 2025
@franziskuskiefer
Copy link
Member

Looks like this fails ml-kem laxing

@franziskuskiefer franziskuskiefer requested a review from W95Psp July 21, 2025 17:46
@Parrot7483
Copy link
Collaborator Author

Parrot7483 commented Jul 21, 2025

This might be the same problem I had with charon dependency. The build script pulls in all FStar files from everywhere, including the ones from ~/.cargo. Charon has its own fork of HAX.

The Libcrux_ml_kem.Vector.Neon.Vector_type.fsti is correct when checking with VSCode interactive mode. I fixed in like this in my sha3 branch.

@W95Psp
Copy link
Collaborator

W95Psp commented Jul 22, 2025

I think this failure is because of #1094.
I will test locally: if that goes through, I will force merge.

@W95Psp
Copy link
Collaborator

W95Psp commented Jul 22, 2025

Indeed, this is working locally, using a git dependency for hax-lib.
Since hax-lib is pinned to a crates.io version, but hax itself is not pinned, this PR will break Libcrux. Thus, either:

  • we change hax-lib to be a git dependency
  • we pin hax itself to the version given by hax-lib

WDYT @franziskuskiefer?

@W95Psp
Copy link
Collaborator

W95Psp commented Jul 22, 2025

Requires cryspen/libcrux#1074 and #1279.

Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

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

The CI should probably be happy now, let's push to merge queue again.

@W95Psp W95Psp added this pull request to the merge queue Jul 24, 2025
Merged via the queue into main with commit 3015005 Jul 24, 2025
17 checks passed
@W95Psp W95Psp deleted the fix-core-clone branch July 24, 2025 06:56
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.

3 participants