Skip to content

Fix regex literal loc #7683

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 3 commits into from
Jul 22, 2025
Merged

Fix regex literal loc #7683

merged 3 commits into from
Jul 22, 2025

Conversation

zth
Copy link
Collaborator

@zth zth commented Jul 20, 2025

This fixes the regex literal locations, so error messages work properly.

No idea if this is the correct fix though. Maybe @shulhi or @cristianoc has opinions?

Closes #7682

@zth zth requested review from cristianoc and shulhi July 20, 2025 11:19
@zth zth force-pushed the fix-regex-syntax-loc branch from b21f017 to 3dbbb44 Compare July 20, 2025 11:20
Copy link

pkg-pr-new bot commented Jul 20, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7683

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7683

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7683

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7683

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7683

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7683

commit: ee43d01

Comment on lines 1818 to 1826
let loc =
mk_loc
{
start_pos with
(* Account for the inserted leading `/` *)
pos_cnum = start_pos.pos_cnum - 1;
}
p.prev_end_pos
in
Copy link
Member

Choose a reason for hiding this comment

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

Would it work if you pass in the start_pos from the caller function in here?

let start_pos = p.Parser.start_pos in

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably! I'll try it when I'm back at the computer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@shulhi yup, much better. Thank you!

@zth zth force-pushed the fix-regex-syntax-loc branch from 172bff5 to 1cb5bf3 Compare July 21, 2025 16:39
@zth zth requested a review from shulhi July 21, 2025 16:42
Copy link
Member

@shulhi shulhi left a comment

Choose a reason for hiding this comment

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

LGTM!

@zth zth force-pushed the fix-regex-syntax-loc branch from 1cb5bf3 to ee43d01 Compare July 22, 2025 12:01
@zth zth enabled auto-merge (squash) July 22, 2025 12:01
@zth zth merged commit e7f8e3a into master Jul 22, 2025
27 checks passed
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.

Regex syntax locations are broken
2 participants