Skip to content

unicode/norm: compose non-Hangul pairs inside a Hangul buffer - #70

Open
tannevaled wants to merge 1 commit into
golang:masterfrom
tannevaled:norm-hangul-buffer-drops-composition
Open

unicode/norm: compose non-Hangul pairs inside a Hangul buffer#70
tannevaled wants to merge 1 commit into
golang:masterfrom
tannevaled:norm-hangul-buffer-drops-composition

Conversation

@tannevaled

@tannevaled tannevaled commented Aug 25, 2026

Copy link
Copy Markdown

A V or T jamo anywhere in the reorder buffer diverts the whole segment to
combineHangul, and its default arm copies a character through without ever
consulting the composition table. Anything in that segment that is not an
L+V or LV+T pair therefore stops composing.

norm.NFC.String("가\U000113C5")
  got  "가\U000113C2\U000113C2"
  want "가\U000113C5"

U+113C5 decomposes to <U+113C2, U+113C2>. U+113C2 combines backward, so it
does not begin a new segment and the pair has to recompose inside the Hangul
buffer, which is where it is lost. The same string composes correctly on its
own and after a starter that does begin a new segment, so the result depends
on unrelated preceding context.

Fall back to combine() before copying through.

Only reachable with the Unicode 16 tables or later: it needs a ccc=0
character that both combines backward and combines forward, and there are
none in Unicode 15 and exactly four from Unicode 16 -- U+113C2, U+1611E,
U+16129 and U+16D67. The added test states an expectation that holds either
way, since a table set without U+113C5 has no decomposition to undo; under
Unicode 15 tables it passes with or without this change, and under
Unicode 17 it fails without it:
it fails without it:

normalize_test.go:613: "가\U000113c5": length is 11; want 7
    ("가\U000113c2{2}" vs "가\U000113c5")

go test -long ./unicode/norm, which runs TestStandardTests and
TestCharacterByCharacter against the real UCD, gives 24079 tests and no
failures under both go1.26.4 and go1.27.0.

Checked beyond the package's own tests: NFC, NFKC and NFD of 2194559
inputs weighted towards Hangul were compared before and after, and each
of the 86234 differing results was judged by a separate UAX #15
implementation built from UnicodeData.txt -- itself checked against
NormalizationTest.txt at 305184 assertions under Unicode 15 and 320544
under Unicode 17 with no failures. 86230 of them are corrections and
none is a regression. The remaining four are golang/go#81001, which is
independent and fixed by CL 821020; with both changes applied all 88743
differences are corrections and none is left.

Fixes golang/go#81021

@gopherbot

Copy link
Copy Markdown
Contributor

This PR (HEAD: 85fdcd5) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/text/+/821040.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

@gopherbot

Copy link
Copy Markdown
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/821040.
After addressing review feedback, remember to publish your drafts!

@tannevaled
tannevaled force-pushed the norm-hangul-buffer-drops-composition branch from 85fdcd5 to bacb127 Compare August 26, 2026 12:31
@gopherbot

Copy link
Copy Markdown
Contributor

This PR (HEAD: bacb127) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/text/+/821040.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

A V or T jamo anywhere in the reorder buffer diverts the whole segment to
combineHangul, and its default arm copies a character through without ever
consulting the composition table. Anything in that segment that is not an
L+V or LV+T pair therefore stops composing.

    norm.NFC.String("가\U000113C5")
      got  "가\U000113C2\U000113C2"
      want "가\U000113C5"

U+113C5 decomposes to <U+113C2, U+113C2>. U+113C2 combines backward, so it
does not begin a new segment and the pair has to recompose inside the Hangul
buffer, which is where it is lost. The same string composes correctly on its
own and after a starter that does begin a new segment, so the result depends
on unrelated preceding context.

Fall back to combine() before copying through.

Only reachable with the Unicode 16 tables or later: it needs a ccc=0
character that both combines backward and combines forward, and there are
none in Unicode 15 and exactly four from Unicode 16 -- U+113C2, U+1611E,
U+16129 and U+16D67. The added test states an expectation that holds either
way, since a table set without U+113C5 has no decomposition to undo; under
Unicode 15 tables it passes with or without this change, and under
Unicode 17 it fails without it:
it fails without it:

    normalize_test.go:613: "가\U000113c5": length is 11; want 7
        ("가\U000113c2{2}" vs "가\U000113c5")

go test -long ./unicode/norm, which runs TestStandardTests and
TestCharacterByCharacter against the real UCD, gives 24079 tests and no
failures under both go1.26.4 and go1.27.0.

Checked beyond the package's own tests: NFC, NFKC and NFD of 2194559
inputs weighted towards Hangul were compared before and after, and each
of the 86234 differing results was judged by a separate UAX golang#15
implementation built from UnicodeData.txt -- itself checked against
NormalizationTest.txt at 305184 assertions under Unicode 15 and 320544
under Unicode 17 with no failures. 86230 of them are corrections and
none is a regression. The remaining four are golang/go#81001, which is
independent and fixed by CL 821020; with both changes applied all 88743
differences are corrections and none is left.

Fixes golang/go#81021
@tannevaled
tannevaled force-pushed the norm-hangul-buffer-drops-composition branch from bacb127 to c2e52ee Compare August 26, 2026 12:35
@gopherbot

Copy link
Copy Markdown
Contributor

This PR (HEAD: c2e52ee) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/text/+/821040.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

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.

x/text/unicode/norm: NFC drops a canonical composition when a Hangul syllable precedes it (U+113C5, Unicode 17 tables)

2 participants