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

feat: add model implementation for UTF8 enc/dec #3961

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

digama0
Copy link
Collaborator

@digama0 digama0 commented Apr 20, 2024

This makes the UTF-8 encode and decode functions have lean definitions, so that we can prove properties about them downstream.

@digama0 digama0 changed the title Utf8 model feat: add model implementation for UTF8 enc/dec Apr 20, 2024
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Apr 20, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 20, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 20, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Apr 20, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Apr 20, 2024

Mathlib CI status (docs):

  • 💥 Mathlib branch lean-pr-testing-3961 build failed against this PR. (2024-04-20 16:04:46) View Log
  • 💥 Mathlib branch lean-pr-testing-3961 build failed against this PR. (2024-04-20 16:25:20) View Log
  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase e4daca8d6b02677115282abff8515a9afb6a2a29 --onto 62cdb51ed5b9d8487877d5a4adbcd4659d81fc6a. (2024-04-22 04:40:21)

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 20, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 20, 2024
@kim-em
Copy link
Collaborator

kim-em commented Apr 22, 2024

Could you add a basic test file using test_extern, e.g. starting with

import Lean.Util.TestExtern

instance : BEq ByteArray where
  beq x y := x.data == y.data

test_extern String.toUTF8 "$£€𐍈" -- representative sample of 1-4 byte characters

and then similarly for the other functions?

@kim-em kim-em added the awaiting-author Waiting for PR author to address issues label Apr 22, 2024
@kim-em kim-em added awaiting-review Waiting for someone to review the PR and removed awaiting-author Waiting for PR author to address issues labels Apr 22, 2024
@kim-em
Copy link
Collaborator

kim-em commented Apr 22, 2024

LGTM, but I'm not the one to merge changes to UTF8 handling!

Copy link
Contributor

@nomeata nomeata left a comment

Choose a reason for hiding this comment

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

Good stuff, minor comments only.

tests/lean/run/utf8英語.lean Show resolved Hide resolved
src/Init/Data/String/Extra.lean Outdated Show resolved Hide resolved
Copy link
Contributor

@nomeata nomeata left a comment

Choose a reason for hiding this comment

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

Looks good. test_extern improvement can be generalized in a separate PR. Happy to merge if you are happy with this.

src/Init/Data/String/Extra.lean Outdated Show resolved Hide resolved
@nomeata nomeata enabled auto-merge April 22, 2024 10:18
@nomeata nomeata added this pull request to the merge queue Apr 22, 2024
Merged via the queue into leanprover:master with commit 70a2394 Apr 22, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Waiting for someone to review the PR breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants