Guard L2 reordering against the maximum implicit level (126)#149
Open
gaoflow wants to merge 1 commit into
Open
Guard L2 reordering against the maximum implicit level (126)#149gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
visual_runs_for_line and the deprecated visual_runs panic with "Level error: OutOfRangeNumber" when a line sits entirely at the maximum implicit level 126: new_lowest_ge_rtl() computes 126 | 1 == 127, which exceeds the ceiling. That level is reachable by ordinary text (nesting isolates to explicit depth 125, then a digit raised to implicit 126 by rule I2), so BidiInfo::visual_runs crashes on valid input for both utf-8 and utf-16. Mirror the guard already in reorder_visual: a uniform even (LTR) level needs no reordering, which also short-circuits before the overflow. Fixes servo#145.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BidiInfo::visual_runspanics withLevel error: OutOfRangeNumberwhen a linesits entirely at the maximum implicit level 126. In the L2 step,
min_level.new_lowest_ge_rtl()evaluatesLevel::new(126 | 1)=Level::new(127),which is past the ceiling, so the following
.expect(...)panics.Level 126 is reachable by ordinary text: nest isolates/embeddings to the explicit
depth 125, then a digit inside is raised to implicit level 126 by rule I2 (as noted
in the issue, implicit resolution tops out at 126). Such a line is uniformly 126,
all even, so nothing needs reordering.
reorder_visualalready guards this withif min == max && min.is_ltr(). Theother two L2 implementations were missing it. This adds the same guard to
visual_runs_for_line(which backsvisual_runsfor both utf-8 and utf-16) andto the deprecated
visual_runs. For a uniform even level the guard matches whatthe existing loop already produces, so behaviour is otherwise unchanged and the
conformance suite still passes.
Repro:
Fixes #145.