Skip to content

Line numbers from TASTy #6542

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

Closed
nicolasstucki opened this issue May 21, 2019 · 7 comments · Fixed by #10363
Closed

Line numbers from TASTy #6542

nicolasstucki opened this issue May 21, 2019 · 7 comments · Fixed by #10363

Comments

@nicolasstucki
Copy link
Contributor

When we have a position on a tree the was loaded from TASTy we have no way to compute line numbers and columns as we only have the offset but no source file.

We have identified the following scenarios where we require the line numbers from TASTy files:

So far the most robust solutions that we have are:

  • Keep the line sizes in the TASTy file. Has a small memory footprint.
  • Keep the actual source in the TASTy file. This would have a large footprint but would also give the possibility of printing better errors for inlined positions.

Which solution should be implemented?

@smarter
Copy link
Member

smarter commented May 21, 2019

Keep the actual source in the TASTy file. This would have a large footprint but would also give the possibility of printing better errors for inlined positions.

I vote for this, but I think we should make the source an optional section of the tasty such that we can still produce some output without crashing if it's stripped for some reason (e.g., proprietary code).

@liufengyun
Copy link
Contributor

As discussed in the meeting, we take the first approach:

Keep the line sizes in the TASTy file. Has a small memory footprint.

liufengyun added a commit to dotty-staging/dotty that referenced this issue Feb 13, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this issue Feb 14, 2020
@liufengyun
Copy link
Contributor

Storing ranges or even source files would require big refactoring of the TASTY infrastructure, as now TASTY chunks are associated with top-level classes. There is no infrastructure to uniquely identify source files and share data (content/line ranges).

The simplest thing to do seems to convert the representation of Span to (line, column) mode during pickling. The conversion can be lossless, i.e. we keep three pairs of (line, column) for start, end and point respectively. The approach keeps the size & information of TASTY files unchanged, and there is no need to change the infrastructure of TASTY. It only requires change Span to be in two different modes: offset mode and line/column mode. However, during a brief discussion, @odersky mentioned this is a wrong approach, for reasons I don't fully understand.

Any ideas @nicolasstucki @smarter ?

@nicolasstucki
Copy link
Contributor Author

I had a branch where I added an extra section with the sizes of the lines of the source file. This was not too difficult.

@liufengyun
Copy link
Contributor

I had a branch where I added an extra section with the sizes of the lines of the source file. This was not too difficult.

In your branch, do you share the line ranges for the same source file across library boundaries?

@nicolasstucki
Copy link
Contributor Author

The lines of the same source. We need to avoid duplication.

@romanowski
Copy link
Contributor

Scala3doc is affected since we've got infrastructure ready to link to line in source file however we are getting -1 as a linie number.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 18, 2020
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 18, 2020
@nicolasstucki nicolasstucki linked a pull request Nov 18, 2020 that will close this issue
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 20, 2020
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 21, 2020
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 21, 2020
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 24, 2020
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Nov 24, 2020
nicolasstucki added a commit that referenced this issue Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants