Skip to content

Unicode: Qualify Base.ismalformed when calling it#61140

Merged
oscardssmith merged 1 commit intomasterfrom
dpa/unicode-ismalformed
Feb 26, 2026
Merged

Unicode: Qualify Base.ismalformed when calling it#61140
oscardssmith merged 1 commit intomasterfrom
dpa/unicode-ismalformed

Conversation

@DilumAluthge
Copy link
Member

ismalformed() is not exported from Base, so we need to qualify it when calling.

Detected by JET.

@stevengj
Copy link
Member

backport?

@DilumAluthge DilumAluthge added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 bugfix This change fixes an existing bug merge me PR is reviewed. Merge when all tests are passing labels Feb 24, 2026
@Keno
Copy link
Member

Keno commented Feb 25, 2026

Does this suggest we do not have tests for this function?

@KristofferC KristofferC mentioned this pull request Feb 25, 2026
36 tasks
@stevengj
Copy link
Member

Yeah, would be good to have a test.

@stevengj
Copy link
Member

stevengj commented Feb 25, 2026

This function is not public, however, and only the ::Integer method is used internally as far as I can tell.

It also doesn't seem to be used in any packages.

But still, might as well have a test.

@stevengj
Copy link
Member

stevengj commented Feb 26, 2026

For example, maybe add a test like:

@test Unicode.combining_class('\u0302') === 0x00e6 # combining class "Above"
@test Unicode.combining_class(reinterpret(Char, UInt32(0xc0) << 24)) === 0x0000 # malformed

@stevengj stevengj removed merge me PR is reviewed. Merge when all tests are passing labels Feb 26, 2026
@oscardssmith oscardssmith merged commit 1cd77b5 into master Feb 26, 2026
12 of 16 checks passed
@oscardssmith oscardssmith deleted the dpa/unicode-ismalformed branch February 26, 2026 03:26
@oscardssmith
Copy link
Member

Oops, saw approval and mergme, and tests passing and clicked merged (and just after realized that the mergme was a removal of mergme not an addition. Can the tests go in a separate PR or should I revert and remake?

@DilumAluthge
Copy link
Member Author

Can the tests go in a separate PR

I think the tests can go in a a separate PR.

@DilumAluthge
Copy link
Member Author

Can the tests go in a separate PR

I think the tests can go in a a separate PR.

Tests are in: #61172

DilumAluthge added a commit that referenced this pull request Mar 1, 2026
KristofferC pushed a commit that referenced this pull request Mar 3, 2026
`ismalformed()` is not exported from `Base`, so we need to qualify it
when calling.

Detected by JET.

(cherry picked from commit 1cd77b5)
KristofferC pushed a commit that referenced this pull request Mar 3, 2026
Follow-up to #61140

(cherry picked from commit 49f889c)
KristofferC pushed a commit that referenced this pull request Mar 3, 2026
`ismalformed()` is not exported from `Base`, so we need to qualify it
when calling.

Detected by JET.

(cherry picked from commit 1cd77b5)
KristofferC pushed a commit that referenced this pull request Mar 3, 2026
Follow-up to #61140

(cherry picked from commit 49f889c)
@KristofferC KristofferC mentioned this pull request Mar 3, 2026
56 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 bugfix This change fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants