Skip to content

compute_caret_offset does not undo the multi-line paren-wrap offset shift #339

Description

@leongdl

Summary

Multi-line expressions are wrapped in parentheses for implicit line continuation (parse.rs, let to_parse = format!("({})", source)). The wrap shifts every offset the parser reports — AST ranges and error locations alike — by +1 relative to the unwrapped expression text.

There is no single place that undoes the shift. Four consumers compensate independently, and one does not compensate at all:

consumer compensates?
Display for ExpressionError (error.rs:314) yes, subtracts 1 from the column when the expression contains a newline
eval_number float passthrough (evaluator.rs:508) yes, subtracts a shift derived the same way before slicing the unwrapped source
the keyword-rename retry (parse.rs) yes, subtracts 1 from the parser's error offset
compute_caret_offset (error.rs:403) no

compute_caret_offset indexes expr.as_bytes() with raw, still-shifted AST offsets while expr is the unwrapped source, so its backwards operator scan reads bytes one position to the right of the intended ones.

Impact

Cosmetic, and limited to the BinOp arm. The caret for a multi-line ** or // lands on the operator's second character:

~~~^     actual
~~^      correct

The Attribute/Call/Subscript arms are unaffected, because they subtract two shifted offsets from each other and the shift cancels.

The message itself is correct in all cases — it already names the operator and both operand types — so this misplaces a caret column rather than misreporting anything.

Existing coverage

There is an #[ignore]d test for this, added in #321, which fails with ~~~^ where ~~^ is correct. Un-ignoring it is the natural verification for a fix.

Suggested fix

Centralize the adjustment rather than adding a fourth independent - 1:

  • record the shift on ParsedExpression at parse time, or
  • normalize AST ranges once after a successful multi-line parse.

Either removes three copies of the same compensation and fixes the caret. Both change the diagnostic output of every multi-line BinOp error, so this needs its own regression tests and a decision about the intended caret placement — which is why it was left out of #321.

Context

Raised in review of #321 (#321 (comment) and the wrap-shift thread), where the reviewer asked that the in-code note be replaced by a tracked issue. The 25-line comment in parse.rs describing this is being reduced to a pointer here.

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