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

Read assertion from file with offsets #513

Closed
wants to merge 4 commits into from
Closed

Read assertion from file with offsets #513

wants to merge 4 commits into from

Conversation

Akuli
Copy link
Owner

@Akuli Akuli commented Jul 28, 2024

The fix for ugly assertion reading. Adds file offsets to tokens, so that fseek() can be used to go to where the assertion starts and ends in the file. This way we can read the file contents after tokenizing.

  • Fix weird error that only happens on Windows
  • Port to self-hosted

@Akuli
Copy link
Owner Author

Akuli commented Jul 28, 2024

This doesn't work, because of the unread_byte() function and Windows CRLF (\r\n) line endings.

When tokenizing, it is possible to unread a byte that was previously read from the file. For example, to read the printf from printf("hi"), you read bytes one at a time until you get a byte that is not valid in a name, in this case (. Then you unread the ( so that the next read will return it.

The problem is unreading "\r\n" (Windows newline), which is translated to '\n' when reading, so that read_byte() consumes the \r and the \n, then returns just the \n. If we unread a \n, then we don't know whether the file contained \n or \r.

I could seek the file to check if it contains a \r before the \n, but then you can't compile unseekable files (such as output of another command), which would be annoying.

A better fix would be to read the entire source code into a string before tokenizing it.

@Akuli Akuli closed this Jul 28, 2024
@Akuli Akuli deleted the offsets branch July 28, 2024 13:46
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.

1 participant