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

last semi column in inside the last expression #7619

Closed
vicuna opened this issue Sep 3, 2017 · 5 comments
Closed

last semi column in inside the last expression #7619

vicuna opened this issue Sep 3, 2017 · 5 comments

Comments

@vicuna
Copy link

vicuna commented Sep 3, 2017

Original bug ID: 7619
Reporter: ChriChri
Status: resolved (set by @xavierleroy on 2017-10-18T15:11:43Z)
Resolution: fixed
Priority: normal
Severity: tweak
Version: 4.05.0
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: lexing and parsing
Monitored by: ChriChri @gasche

Bug description

Compiling the following

let f x y = x y; x y;

Produce the following position for the first application:

          expression (tmp2.ml[1,0+12]..[1,0+15])
            Pexp_apply

And this for the second:

          expression (tmp2.ml[1,0+17]..[1,0+21])
            Pexp_apply

The semicolumn is part of the last application, which look wrong

Steps to reproduce

compile the above line with -dparsetree

@vicuna
Copy link
Author

vicuna commented Sep 29, 2017

Comment author: @xavierleroy

Assuming this is the behavior of this trailing semicolon, how bad is it? who or what piece of code is affected by it? why should someone invest time and effort tracking it down and changing it?

@vicuna
Copy link
Author

vicuna commented Oct 1, 2017

Comment author: ChriChri

Because when writing alternative parser for ocaml, it might be a nightmare to
get the same position as OCaml. And then when you test, it is hard to distinguish
the differences that are important and those that are irrelevant (like this one).

I will try to see if it is easy to fix and submit a pull request ...

@vicuna
Copy link
Author

vicuna commented Oct 1, 2017

Comment author: ChriChri

I think the fix took me 30s ... And will make the position more
consistant with the optional last semicolmun in list or arrays.

@vicuna
Copy link
Author

vicuna commented Oct 1, 2017

Comment author: ChriChri

I submitted a PR on the 4.06.0 branch with

  • removed one reloc_exp from parsing/parser.ml
  • added two lines in Changes
  • fixed the reference for (only) one test that had a useless ';'

I hope I followed the guidelines ...

@vicuna
Copy link
Author

vicuna commented Oct 18, 2017

Comment author: @xavierleroy

#1387 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant