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: Rust-style raw string literals #2929

Merged
merged 6 commits into from
Dec 20, 2023
Merged

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Nov 20, 2023

For example, r"\n" and r#"The word "this" is in quotes."#.

Implements #1422

@kmill kmill added the awaiting-review Waiting for someone to review the PR label Nov 20, 2023
@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 Nov 20, 2023
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Nov 20, 2023

  • ❗ Mathlib CI will not be attempted unless you rebase your PR onto the 'nightly' branch. (2023-11-20 21:36:37)
  • ✅ Mathlib branch lean-pr-testing-2929 has successfully built against this PR. (2023-11-28 20:27:46) View Log
  • ❗ Mathlib CI can not be attempted yet, as the 'nightly-testing-2023-12-19' branch does not exist there yet. We will retry when you push more commits. It may be necessary to rebase onto 'nightly' tomorrow. (2023-12-19 17:42:28)

@hargoniX
Copy link
Contributor

Is there a particular reason (apart from copying Rust syntax) that this breaks the current Lean convention of having modifiers before strings with exclamation marks (think s! and m!). I feel like it would be rather confusing for a new user that some modifiers come with ! and others with #.

@kmill
Copy link
Collaborator Author

kmill commented Nov 26, 2023

@hargoniX One perspective here is that interpStr starts with !", and then letters are used to give what kind of interpolated string it is. Here, r plays the role of ! but for rawStr. The #'s are not playing the role of ! -- r"\n" is a raw string too.

That's just a possible interpretation of the current situation, though I think the fact we only have interpStr modifiers is worth considering. Switching from r to r! could be justifiable, if we do want to go that way.

src/Lean/Parser/Basic.lean Outdated Show resolved Hide resolved
@kmill
Copy link
Collaborator Author

kmill commented Dec 12, 2023

@Kha I think this is ready for review

@Kha
Copy link
Member

Kha commented Dec 19, 2023

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 205aa90.
There were no significant changes against commit 78200b3.

@Kha
Copy link
Member

Kha commented Dec 19, 2023

There is a ~1% parser run time regression. That sounds acceptable, but it might be worth a try avoiding the allocation for the rawStrLitStart result by only calling it when we're looking at an r. Makes sense?

@kmill
Copy link
Collaborator Author

kmill commented Dec 19, 2023

@Kha I modified it to avoid allocation and to use the cheap curr == 'r' check. I also simplified the raw string parser state machine by making the states more explicit, which also saves some branching. One thing I decided is that it is OK to do lookahead for the initial ##...#" for a fast check and then parse it again to actually count the #'s, since then this fast check definitely does not need to allocate anything.

I'll run the benchmarks again once the build succeeds.

@kmill
Copy link
Collaborator Author

kmill commented Dec 19, 2023

!bench

@tydeu
Copy link
Member

tydeu commented Dec 19, 2023

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 781869c.
There were no significant changes against commit 67bfa19.

@Kha Kha added this pull request to the merge queue Dec 20, 2023
Merged via the queue into leanprover:master with commit ae6fe09 Dec 20, 2023
7 checks passed
@semorrison semorrison mentioned this pull request Jan 13, 2024
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 builds-mathlib CI has verified that Mathlib builds against this PR 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.

None yet

8 participants