Skip to content

fix(ir/irverify): read blank names as Unicode defines them - #1513

Open
fuad-daoud wants to merge 14 commits into
mainfrom
fix/irverify-blank-names-unicode
Open

fuad-daoud wants to merge 14 commits into
mainfrom
fix/irverify-blank-names-unicode

Conversation

@fuad-daoud

@fuad-daoud fuad-daoud commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

In short: irverify decides whether a name is blank with its own copy of Unicode's list of invisible characters, and the copy was wrong for 292 code points. This makes it follow Unicode's definition: an entry of variation selectors only is now reported blank, and visible format characters such as U+0600 no longer are.

isInvisible approximated Unicode's Default_Ignorable_Code_Point as space, control, Cf and Other_Default_Ignorable_Code_Point, which is wrong in both directions.

  • It missed Variation_Selector, so an entry made only of selectors was not blank.
  • It counted the Cf characters Unicode takes out of the property because they render: Prepended_Concatenation_Mark, the interlinear annotation characters U+FFF9..FFFB and the Egyptian hieroglyph format controls U+13430..1343F. An entry made of those was reported blank.

It now follows the property's definition. Package unicode has a table for every term but those two ranges, which are named constants. Both callers of isBlankName change with it: the alias rule (ir/naming-alias-blank) and the string-list rules (ir/path-segment-blank, ir/set-entry-blank). docs/ir/naming.md states the new set.

The only one of these lists a compiler writes is Visibility.Only, from fixed lifecycle names, so no spec's output moves, and the IR's shape does not change.

Limitation: the two constants hold for Unicode 17.0.0, as no unicode table carries the property. TestIsInvisible_RangesWereCheckedAgainstThisUnicode fails when a Go release moves unicode.Version, so that move rechecks them against DerivedCoreProperties.txt. That file's header writes the second range as 13430..13440; U+13440 is Mn and in no added term, so subtracting it changes nothing. No open issue tracks this. #1511, closed, left a test tying a predicate to unicode.Version out of its own scope; this one pins the version string alone, and the sweep over the UCD files stays a throwaway probe, as .claude/skills/morphic-changing-the-ir/references/ir-lessons.md#sweep-a-published-property prescribes.

Closes #832

Before and after

flowchart TB
  subgraph before ["Before"]
    b0["An alias or list entry of variation selectors only"]
    b1["Passes as a name, though it names nothing"]
    b2["An entry of U+0600 or U+FFF9, which render, is reported blank"]
    b0 --> b1
    b1 --> b2
  end
  subgraph after ["After"]
    a0["An alias or list entry of variation selectors only"]
    a1["Reported blank by irverify"]
    a2["An entry of U+0600 or U+FFF9 passes as visible"]
    a0 --> a1
    a1 --> a2
  end
  before ~~~ after
Loading

What it brings: irverify's blank-name rules now hold Default_Ignorable_Code_Point as Unicode publishes it on every code point, instead of a near copy that was wrong for 292 of them. White space and controls stay invisible through their own terms, as before (#832), U+1680 OGHAM SPACE MARK among them though it renders a line.

Test plan

  • CI on this head, main at 2123d296 merged in, runs make gate. On 57484050, go build ./..., go test ./internal/archtest, go test -cover ./ir/irverify at 100.0%, make vet, make lint, make lint-planted and the skills lint each pass.
  • Every code point against Unicode 17.0.0's DerivedCoreProperties.txt and PropList.txt, by a throwaway in-package test: visited 1114112 ucdDiffs 0. Against the predicate on main, 260 code points are added, all Variation_Selector, and 32 removed, the characters listed above.
  • Removing each term of isInvisible, one at a time, reddens a row. So does moving a bound inward, or interlinearFirst to U+FFF8. Rows for U+13440 and U+E000 keep Variation_Selector and Cf from widening to Mn, M or C, and rows for U+FFFC and U+1342F keep Cf from widening to So or Lo: each of those mutants reddens one. Narrowing the white-space term to exclude U+1680 reddens the ogham space mark row. Narrowing it to Zs reddens the line separator and paragraph separator rows. Narrowing the control term to C0 reddens delete and c1 control, and to C0 plus DEL, c1 control. Narrowing the Other_Default_Ignorable_Code_Point term with && !unicode.Is(unicode.Mn, r) reddens the combining grapheme joiner row. Three bound moves stay green and are equivalent, because the code points they add are false either way: interlinearLast to U+FFFC, egyptianFormatFirst to U+1342F and egyptianFormatLast to U+13440.
  • sweep.py -b origin/main -j 2 testdata -- --verify over 236 specs: "no spec's exit code, IR or diagnostics differ". With isInvisible planted to answer true, 4 specs go from exit 0 to 2; without --verify no compile runs irverify, and the same plant shows nothing.
  • TestIRVersion_PinsTheEncodedShape and TestIRVersion_PinsTheNames stay green at an unmoved IRVersion.

isInvisible approximated Default_Ignorable_Code_Point as space, control,
Cf and Other_Default_Ignorable_Code_Point, wrong in both directions. It
missed Variation_Selector (U+FE00..FE0F, U+E0100..E01EF, U+180B..180D,
U+180F), so an entry made only of selectors was not blank. It counted
the Cf characters Unicode takes out of the property because they render:
Prepended_Concatenation_Mark, the interlinear annotation characters
U+FFF9..FFFB and the Egyptian hieroglyph format controls
U+13430..1343F, so an entry made of those was reported blank.

Add Variation_Selector and subtract the other three. Package unicode has
a table for every term but the two ranges, which are named constants.
Both callers of isBlankName change with it: the alias rule
(ir/naming-alias-blank) and the string-list rules
(ir/path-segment-blank, ir/set-entry-blank). No IR shape changes.
A test now holds the interlinear exclusion's lower bound, with U+FFF8 as
the witness directly below it.

The test's comment no longer calls the format characters graphic.

The constants name the Unicode version they hold for, and say to
recheck them when unicode.Version moves.
The two range tests move into locals so the case stays on one line, as
the styleguide's rule 1.8 asks. The predicate answers the same on every
code point.
@fuad-daoud
fuad-daoud marked this pull request as draft November 5, 2026 20:23
@fuad-daoud
fuad-daoud removed the request for review from OmarAlJarrah November 5, 2026 20:23
Rows just outside the interlinear and Egyptian format ranges and a private use character keep Variation_Selector and Cf from widening unseen. The test is renamed for what its rows share, and a second pins the ranges to unicode.Version, which no table of package unicode carries.
The visible-alias rows keep a term from widening over a bound, not a
bound from widening: an exclusion moved outward can only answer false,
so a false row cannot see it. Say so in the test's comment.

Rename the unicode.Version pin to say the ranges were checked, not that
they hold, and have its failure say what to recheck. Record beside the
constants why the second range stops at U+1343F where
DerivedCoreProperties.txt's header writes U+13440. Correct a comment on
the blank-list test that still counted one visible entry.
The blank-name comments and docs/ir/naming.md said Unicode classifies
white space as invisible, but U+1680 OGHAM SPACE MARK renders a visible
line. The rules take white space, controls and
Default_Ignorable_Code_Point as one set the package chose, with U+1680
counting as white space; say so. No behaviour changes.
@fuad-daoud
fuad-daoud marked this pull request as ready for review November 5, 2026 23:26
@fuad-daoud
fuad-daoud marked this pull request as draft November 6, 2026 08:45
@fuad-daoud
fuad-daoud removed the request for review from OmarAlJarrah November 6, 2026 08:45
White_Space reaches beyond Zs and Cc: U+2028 and U+2029 are neither, so the
rows only the space term held were all Zs. Controls past C0 had no witness
either, so narrowing the control term to C0 stayed green. Add a row for each
of U+2028, U+2029, DEL and a C1 control.
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.

irverify: the blank-alias rule misreads Unicode's default-ignorable set

1 participant