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

reporting: Start span start line mis-calculation #299

Closed
feds01 opened this issue Jun 19, 2022 · 3 comments · Fixed by #303
Closed

reporting: Start span start line mis-calculation #299

feds01 opened this issue Jun 19, 2022 · 3 comments · Fixed by #303
Assignees
Labels
bug Something isn't working error-reporting Error reporting sub-system issues

Comments

@feds01
Copy link
Contributor

feds01 commented Jun 19, 2022

In the following error report:
image

The highlighter mistakingly highlights the 9 line number. The span of the expression does not actually begin on the 9th line. This might be a mistake within the lexing implementation or a miscalculation within the report offset to (col, row) calculation. It's clear that the line of the span is for some reason being recorded as the end of line 9 where there is a \n character at the end.

Further investigation yielded that the actual span of expression (as printed from the raw source is):

loc=134:195, actual="IoError = struct(
    error: IoErrorType,
    message: str,
);"

Ideally, the report should not highlight the line number 9, like so:

image

This could be fixed by changing offset_col_row by not counting the \n in the case of the initial span offset, an implementation could be:

pub(crate) fn offset_col_row(offset: usize, source: &str, non_inclusive: bool) -> (/* col: */ usize, /* row: */ usize) {
    let source_lines = source.split('\n');

    let mut bytes_skipped = 0;
    let mut total_lines: usize = 0;
    let mut last_line_len = 0;

    let mut line_index = None;
    for (line_idx, line) in source_lines.enumerate() {
        // One byte for the newline
        let skip_width = line.len() + 1;


        // Here, we don't want an inclusive range because we don't want to get the last byte because
        // that will always point 
        let range = if non_inclusive {
            bytes_skipped..bytes_skipped + skip_width
        } else {
            bytes_skipped..bytes_skipped + skip_width + 1
        };

        if range.contains(&offset) {
            line_index = Some((offset - bytes_skipped, line_idx));
            break;
        }

        bytes_skipped += skip_width;
        total_lines += 1;
        last_line_len = line.len();
    }

    line_index.unwrap_or((
        last_line_len.saturating_sub(1),
        total_lines.saturating_sub(1),
    ))
}
@feds01 feds01 added bug Something isn't working error-reporting Error reporting sub-system issues labels Jun 19, 2022
@feds01 feds01 self-assigned this Jun 19, 2022
@kontheocharis
Copy link
Collaborator

What determines if a report span should be non_inclusive?

@feds01
Copy link
Contributor Author

feds01 commented Jun 19, 2022

When the start_row, start_col is being computed

@kontheocharis
Copy link
Collaborator

It would be interesting to see the col:row start and end points for this example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working error-reporting Error reporting sub-system issues
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants