Skip to content

Non-ASCII text before an error panics prqlc or shifts the error label (byte vs char spans) #6377

Description

@prql-bot

A query with non-ASCII text before an error makes prqlc panic, or put the error label in the wrong place:

$ printf 'from t\nderive {x = "éééééééééé"}\nselect {x, foo.bar.baz}\n' | prqlc compile
The application panicked (crashed).
Message:  span Some(1:54-65) is out of bounds of the source (len = 57)
Location: prqlc/prqlc/src/error_message.rs:153

Reproduced on 93ab9fc. With fewer multi-byte characters it doesn't panic, but the label moves: after # café on the first line, the "Unknown name" label for a later column sits one column to the right of the name. prqlc debug annotate places frames with the same spans, so it can attach them to the wrong lines.

Cause. Tokens and error spans use different units.

  • The lexer builds token spans straight from chumsky's SimpleSpan (lexer/mod.rs:130-148). Over &str input, those are byte offsets.
  • Everything downstream reads spans as char offsets: ariadne's Source, and the lexer's own error path, convert_lexer_error, which converts byte offsets to char offsets on purpose.
  • So every multi-byte character shifts later spans right by its extra bytes. Once a span runs past the source's char length, the assert! in error_message.rs panics.

The chumsky 0.10 migration (#5223) appears to be where token spans became byte offsets.

Why this needs a decision. One of two conventions has to be picked, and spans are visible through the JS, Python and C bindings:

  1. Char offsets everywhere. Convert token spans in the lexer's map_with closures, as convert_lexer_error already does. This needs a byte→char table built once per source; a per-token chars().count() would be quadratic.
  2. Byte offsets everywhere. Switch ariadne to byte indexing (Config::with_index_type(IndexType::Byte)), drop the conversion in convert_lexer_error, and check the other consumers of spans, including the bindings and debug annotate.

Separately, the assert! in error_message.rs turns a bad span into a crash. Falling back to an error with no location would keep a span bug from taking the process down.

Found during the nightly code-quality survey.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions