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

parser.mly: consistently use $sloc over $loc #2031

Merged
merged 1 commit into from Sep 8, 2018

Conversation

gasche
Copy link
Member

@gasche gasche commented Sep 7, 2018

Menhir has two keywords to describe "the current source position",
$loc and $sloc. $sloc is more precise (it is included within $loc),
and the two differ only when the production starts with empty symbols,
and the difference only spans over whitespace.

We originally used $loc by default to emulate ocamlyacc behavior,
but $sloc is generally preferable, so this PR converts all locations
to use $sloc.

(Note: the location-of-a-symbol keyword remains $loc(...), for example
$loc($1) or $loc(foo), there is no $sloc version of those.)

I have tested this PR using #2030, and it does not, in fact, change any parse location in the compiler distribution. On the contrary, the converse change (turning all $sloc into $loc) produces a lot of location changes, as expected.

This proposed change was mentioned/discussed in #2029 and, more importantly, in #2016 (comment).

Menhir has two keywords to describe "the current source position",
$loc and $sloc. $sloc is more precise (it is included within $loc),
and the two differ only when the production starts with empty symbols,
and the difference only spans over whitespace.

We originally used $loc by default to emulate ocamlyacc behavior,
but $sloc is generally preferable, so this PR converts all locations
to use $sloc.

(Note: the location-of-a-symbol keyword remains $loc(...), for example
$loc($1) or $loc(foo), there is no $sloc version of those.)
Copy link
Contributor

@nojb nojb 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!

@gasche gasche merged commit cebf9c1 into ocaml:trunk Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants