Fix case mapping, Hangul composition, and property table bugs - #20
Merged
Conversation
Fixes seven bugs found by review, several of which corrupt data or crash: - compose_hangul off-by-one on both trailing-consonant bounds: NFC silently deleted U+11A7 after an LV syllable and composed U+11C3 into the wrong syllable (e.g. AC00 11C3 -> AC1C). Also tighten is_decomposed_syllable to exclude TBase itself. - is_more_above scanned from i - 1 instead of i + 1, reading s32[-1] at the start of the string (caught by ASan) and testing the wrong characters for the Lithuanian More_Above context. - is_before_dot / is_more_above returned true when the scan hit the end of text without finding the target, so Turkish to_lowercase(U"I", "tr") produced "i" instead of dotless "ı". - full_case_mapping had no switch case for language-qualified SpecialCasing entries without a context condition (SpecialCasingContext::Unassigned), so they were never applied: Lithuanian Ì/Í/Ĩ and Turkish İ -> i did not work. - full_case_mapping crashed on mappings to the empty string (stored as null, e.g. U+0307 removed after 'I' in Turkish); append nothing instead of dereferencing the null pointer. - is_standard_korean_syllable_block used || instead of && in its trailing-consonant loop, absorbing arbitrary following characters into the block and reading one code point past the end. - gen_tables.py assigned property bits by order of appearance in PropList.txt, but the Property_* constants in unicodelib.h are fixed. Unicode 17 inserted IDS_Unary_Operator and the ID_Compat_Math properties, shifting every bit from Radical onward: is_radical through is_prepended_concatenation_mark (13 predicates, including is_soft_dotted used by case mapping) all tested the wrong bit. Bits are now assigned from fixed maps that match the header constants (DerivedCoreProperties likewise, which currently matches), and the _properties table is regenerated. Adds regression tests for Turkish/Azeri and Lithuanian case mappings, Hangul composition boundary values, Korean syllable block scanning, and one probe per previously misaligned property. Claude-Session: https://claude.ai/code/session_01LfmQizYVn2aWEovhNfzXG4
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
1 participant
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.
Summary
Fixes seven bugs found during a code review of the hand-written logic in
unicodelib.h, several of which corrupt data or crash. All were verified with small repro programs (AddressSanitizer clean after the fixes) and are covered by new regression tests.Data corruption / crashes
compose_hangulacceptedTIndex == 0andTIndex == TCount, soNFC(U+AC00 U+11A7)silently deleted U+11A7 andNFC(U+AC00 U+11C3)composed into the wrong syllable (AC1C 개). Both bounds now match UAX Improveutf8::decode()andutf16::decode()#15 (0 < TIndex && TIndex < TCount), andis_decomposed_syllableno longer treats TBase as a valid trailing consonant.is_more_above— it scanned fromi - 1instead ofi + 1, readings32[-1]at the start of the string (caught by ASan viato_lowercase(U"Ì", "lt")) and evaluating the Lithuanian More_Above context against the wrong characters.Iin Turkish) are stored as null pointers;full_case_mappingappended them to astd::u32stringand segfaulted.is_standard_korean_syllable_block—||instead of&&in the trailing-consonant loop absorbed arbitrary following characters into the syllable block and read one code point past the end.Wrong results
I→ıbroken —is_before_dot(andis_more_above) returnedtruewhen the scan reached end of text without finding the target, so theNot_Before_Dotcondition failed andto_lowercase(U"I", "tr")returned"i"instead of"ı".SpecialCasingContext::Unassigned, which fell into the switch's unreachabledefault. LithuanianÌ/Í/Ĩand Turkishİ → idid not work at all.gen_tables.pyassigned_propertiesbits by order of appearance inPropList.txt, while theProperty_*constants in the header are fixed. Unicode 17 insertedIDS_Unary_Operatorand theID_Compat_Math_*properties, shifting every bit fromRadicalonward:is_radicalthroughis_prepended_concatenation_mark— includingis_soft_dotted, which case mapping depends on — all returned values for a different property. The generator now uses fixed name→bit maps that match the header constants (same treatment forDerivedCoreProperties, which currently happens to match), and the_propertiestable is regenerated.Tests
New regression tests cover Turkish/Azeri and Lithuanian case mappings, Hangul composition boundary values, Korean syllable block scanning, and one probe per previously misaligned property predicate. Full suite (including the UCD
NormalizationTest.txt/ segmentation test data) passes.🤖 Generated with Claude Code
https://claude.ai/code/session_01LfmQizYVn2aWEovhNfzXG4