Skip to content

IndexOutOfBounds in SourceFile.scala contents() #14795

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
Xavientois opened this issue Mar 27, 2022 · 2 comments · Fixed by #15121
Closed

IndexOutOfBounds in SourceFile.scala contents() #14795

Xavientois opened this issue Mar 27, 2022 · 2 comments · Fixed by #15121
Assignees
Labels
stat:needs minimization Needs a self contained minimization

Comments

@Xavientois
Copy link
Contributor

Xavientois commented Mar 27, 2022

Given a SourcePosition pos where pos.exists and pos.source.exists, I would expect that it is safe to call startColumnPadding on pos. This is not always the case, as can be seen when building the community lib Lucre here.

This exception issue occurs at startColumnPadding in SourceFile.scala, when content() is empty, but length is non-zero.

It seems content().length != length happens when length is computed from Tasty using setLineIndicesFromLineSizes instead of using calculateLineIndicesFromContents.

Originally posted by @olhotak in #14683 (comment)

@nicolasstucki nicolasstucki added the stat:needs minimization Needs a self contained minimization label Mar 28, 2022
@Xavientois
Copy link
Contributor Author

This issue was also identified here: #11210 (comment)

@olhotak
Copy link
Contributor

olhotak commented Apr 6, 2022

#14683 fixed startColumnPadding by replacing length with content().length, so we can probably close this.

Before #10363, length and content().length were synonyms and various functions in SourceFile called length to get content().length. #10363 made them different. Over time, various functions that called length were changed to call content().length instead. Now only one reference to length remains, in offsetToLine.

For future users, it would be good to document the intended meaning of length and how it now differs from content().length, to give a hint as to when to use which one. I would add such a comment myself but I don't fully understand the subtleties of the intended meaning of the new length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat:needs minimization Needs a self contained minimization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants