Upstream the Unicode database table generator and update the tables to 15 #8617

rikkimax · 2022-10-31T10:50:30Z

Right let's see how much this breaks.

dlang-bot · 2022-10-31T10:50:32Z

Thanks for your pull request and interest in making D better, @rikkimax! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8617"

burner · 2022-11-01T12:35:20Z

Considering the size alone of this PR. I would welcome a somewhat long description of what this PR tries to do.
I think I have an idea (update the unicode in phobos), but I might just be overconfident.

rikkimax · 2022-11-01T12:42:09Z

Considering the size alone of this PR. I would welcome a somewhat long description of what this PR tries to do. I think I have an idea (update the unicode in phobos), but I might just be overconfident.

The goal is to get the Unicode table generator into Phobos where it belongs.

From what I can tell, it has never been updated and could be ~8 releases out of date.

Unfortunately, people went messing with the tables since then, and not all changes got committed to the generator so it's not quite as simple as just adding it.

The quality of the generator doesn't matter, nor do the tables themselves. As long as it passes the test suite we are good (Dmitry attempted to update the tables to 14 already, and this approach was the outcome of that).

std/internal/unicode_table_generator.d

ljmf00 · 2022-11-01T12:48:34Z

std/internal/unicode_table_generator.d

+    string[string] aliases;
+}
+
+PropertyTable general;


no need to make them thread-local, it bloats the TLS and its effectively global state.

std/internal/unicode_table_generator.d

rikkimax · 2022-11-01T13:18:35Z

@ljmf00 I didn't develop the generator, that was Dmitry Olshansky ~2013. Its quality does not matter as long as it works. If we do not get the generator into Phobos std.uni is effectively dead.

Right now I want to change as little as possible, it needs to work, beyond that it's out of scope of the PR.

ljmf00 · 2022-11-01T13:40:26Z

@ljmf00 I didn't develop the generator, that was Dmitry Olshansky ~2013. Its quality does not matter as long as it works. If we do not get the generator into Phobos std.uni is effectively dead.

Right now I want to change as little as possible, it needs to work, beyond that it's out of scope of the PR.

I didn't request changes tho, just suggestions :) Some scope and DIP1000 changes may help in discovering memory corruption.

rikkimax · 2022-11-01T13:46:17Z

@ljmf00 I didn't develop the generator, that was Dmitry Olshansky ~2013. Its quality does not matter as long as it works. If we do not get the generator into Phobos std.uni is effectively dead.
Right now I want to change as little as possible, it needs to work, beyond that it's out of scope of the PR.

I didn't request changes tho, just suggestions :) Some scope and DIP1000 changes may help in discovering memory corruption.

All good, I just don't want people wasting their effort on this PR for code review, that stuff can come later once the harder unicode parts are resolved.

source\std\uni\package.d(1534,57): Error: cannot take address of local `idxArray` in `@safe` function `__unittest_L1531_C7`

I'm getting these errors, I'd love it if someone could verify if std.uni is even getting unittested right now.

rikkimax · 2022-11-02T03:38:06Z

Okay looks like the testsuite has got the brilliant idea that it needs to run the generator.

std.exception.ErrnoException@std/stdio.d(547): Cannot open file ucd-15/Blocks.txt' in mode rb' (No such file or directory)

But otherwise its looking like its passing :D

ibuclaw · 2022-11-02T05:26:09Z

Okay looks like the testsuite has got the brilliant idea that it needs to run the generator.

std.exception.ErrnoException@std/stdio.d(547): Cannot open file ucd-15/Blocks.txt' in mode rb' (No such file or directory)

But otherwise its looking like its passing :D

The generator should sit outside of the std source tree, such as in a tools directory.

posix.mak

ibuclaw

Can't see anywhere else that generated code would fall foul of the crude style checks.

tools/unicode_table_generator.d

rikkimax · 2022-11-02T08:52:18Z

Buildkite is failing due to a bug in mir-algorithm, so it's safe to assume as of right now, that this is green!

posix.mak

ibuclaw · 2022-11-02T10:51:11Z

Buildkite is failing due to a bug in mir-algorithm, so it's safe to assume as of right now, that this is green!

It's only failing with this PR though, so perhaps you've indirectly affected something?

https://github.com/libmir/mir-algorithm/blob/aa1e914663999eb94d89e84a30c645ea94def179/source/mir/format.d#L909-L918

ibuclaw · 2022-11-02T10:53:39Z

Buildkite is failing due to a bug in mir-algorithm, so it's safe to assume as of right now, that this is green!

It's only failing with this PR though, so perhaps you've indirectly affected something?

https://github.com/libmir/mir-algorithm/blob/aa1e914663999eb94d89e84a30c645ea94def179/source/mir/format.d#L909-L918

Answer, yes: see Mir's implementation of print

https://github.com/libmir/mir-algorithm/blob/e0acce4313a26385d2e461f58ec5a1a7a6a3acc1/source/mir/format.d#L882-L906

So I'd consider you to be the owner of fixing Mir if you're going to break downstream. :-)

rikkimax · 2022-11-02T10:56:29Z

Buildkite is failing due to a bug in mir-algorithm, so it's safe to assume as of right now, that this is green!

It's only failing with this PR though, so perhaps you've indirectly affected something?
https://github.com/libmir/mir-algorithm/blob/aa1e914663999eb94d89e84a30c645ea94def179/source/mir/format.d#L909-L918

Answer, yes: see Mir's implementation of print

https://github.com/libmir/mir-algorithm/blob/e0acce4313a26385d2e461f58ec5a1a7a6a3acc1/source/mir/format.d#L882-L906

So I'd consider you to be the owner of fixing Mir if you're going to break downstream. :-)

Yeah it's calling into isGraphical, and the character in question wasn't allocated before 7 so it couldn't be graphical in previous tables.

Should be an easy fix, remove one character from the unittest methodology effectively.

changelog/unicode_table_generator.dd

rikkimax · 2022-11-03T08:24:32Z

Looks like I'll have to remove myself as a code owner since I don't have write permissions

RazvanN7 · 2022-11-13T09:34:15Z

cc @atilaneves . Any thoughts on this?

atilaneves · 2022-11-15T12:39:13Z

I agree with @ibuclaw in that the generator should probably be in tools but it seems like it is already?

rikkimax · 2022-11-15T21:33:37Z

I agree with @ibuclaw in that the generator should probably be in tools but it seems like it is already?

It wasn't originally, but I applied that change since it made sense.

The question is can we merge? I wanna get Turkic support into caseless matching ;)

rikkimax · 2022-12-11T06:24:32Z

std/traits.d(8779): Error: static assert: 1LU == 2LU is false

what the?

I see in another PR has this exact issue for the same failed tests. So not my fault, phew.

RazvanN7 · 2022-12-11T09:27:06Z

Rebasing to latest master will fix this.

rikkimax · 2022-12-11T13:00:51Z

Oh dangit I shouldn't have let SourceTree merge branches like that. Oh well, I'll let CI run and confirm the fix for prior error.

…ables

tools/unicode_table_generator.d

RazvanN7 · 2022-12-15T13:07:10Z

@atilaneves @ibuclaw any other objections or is this good to go?

rikkimax requested review from burner and JackStouffer as code owners November 1, 2022 10:22

ljmf00 reviewed Nov 1, 2022

View reviewed changes

std/internal/unicode_table_generator.d Outdated Show resolved Hide resolved

ljmf00 reviewed Nov 1, 2022

View reviewed changes

rikkimax requested review from CyberShadow, MartinNowak and wilzbach as code owners November 2, 2022 04:31

ibuclaw reviewed Nov 2, 2022

View reviewed changes

posix.mak Outdated Show resolved Hide resolved

posix.mak Outdated Show resolved Hide resolved

ibuclaw reviewed Nov 2, 2022

View reviewed changes

tools/unicode_table_generator.d Outdated Show resolved Hide resolved

rikkimax mentioned this pull request Nov 2, 2022

U+23F4 is graphical as of Unicode v7 libmir/mir-algorithm#445

Closed

ibuclaw reviewed Nov 2, 2022

View reviewed changes

posix.mak Outdated Show resolved Hide resolved

ibuclaw reviewed Nov 2, 2022

View reviewed changes

posix.mak Outdated Show resolved Hide resolved

rikkimax force-pushed the unicode_tables branch from 5be3b7f to 6cddb97 Compare November 3, 2022 08:16

ibuclaw reviewed Nov 3, 2022

View reviewed changes

changelog/unicode_table_generator.dd Outdated Show resolved Hide resolved

rikkimax force-pushed the unicode_tables branch from 6cddb97 to 8bd17c6 Compare November 3, 2022 08:21

rikkimax force-pushed the unicode_tables branch 2 times, most recently from 14f643e to 4145185 Compare November 3, 2022 08:29

DmitryOlshansky mentioned this pull request Nov 22, 2022

Unicode 14.0.0 support (tables only) #7469

Closed

rikkimax added 3 commits December 12, 2022 02:10

Upstream Unicode table generator and update tables to v15

122df92

Make Unicode tables module headers consistent and add DO NOT EDIT to it

a1f2e76

Add URL for Unicode table generator into the headers of the Unicode t…

297370a

…ables

rikkimax force-pushed the unicode_tables branch from dacdd9d to 297370a Compare December 11, 2022 13:12

ibuclaw reviewed Dec 11, 2022

View reviewed changes

tools/unicode_table_generator.d Outdated Show resolved Hide resolved

Emphasize DO NOT EDIT for the generated Unicode tables

e848d44

RazvanN7 approved these changes Dec 15, 2022

View reviewed changes

RazvanN7 added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label Dec 15, 2022

RazvanN7 merged commit 67d4521 into dlang:master Dec 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstream the Unicode database table generator and update the tables to 15 #8617

Upstream the Unicode database table generator and update the tables to 15 #8617

rikkimax commented Oct 31, 2022

dlang-bot commented Oct 31, 2022 •

edited

Loading

burner commented Nov 1, 2022

rikkimax commented Nov 1, 2022

ljmf00 Nov 1, 2022

rikkimax commented Nov 1, 2022

ljmf00 commented Nov 1, 2022

rikkimax commented Nov 1, 2022 •

edited

Loading

rikkimax commented Nov 2, 2022

ibuclaw commented Nov 2, 2022

ibuclaw left a comment

rikkimax commented Nov 2, 2022

ibuclaw commented Nov 2, 2022

ibuclaw commented Nov 2, 2022 •

edited

Loading

rikkimax commented Nov 2, 2022

rikkimax commented Nov 3, 2022

RazvanN7 commented Nov 13, 2022

atilaneves commented Nov 15, 2022

rikkimax commented Nov 15, 2022 •

edited

Loading

rikkimax commented Dec 11, 2022

RazvanN7 commented Dec 11, 2022

rikkimax commented Dec 11, 2022

RazvanN7 commented Dec 15, 2022

Upstream the Unicode database table generator and update the tables to 15 #8617

Upstream the Unicode database table generator and update the tables to 15 #8617

Conversation

rikkimax commented Oct 31, 2022

dlang-bot commented Oct 31, 2022 • edited Loading

Bugzilla references

Testing this PR locally

burner commented Nov 1, 2022

rikkimax commented Nov 1, 2022

ljmf00 Nov 1, 2022

Choose a reason for hiding this comment

rikkimax commented Nov 1, 2022

ljmf00 commented Nov 1, 2022

rikkimax commented Nov 1, 2022 • edited Loading

rikkimax commented Nov 2, 2022

ibuclaw commented Nov 2, 2022

ibuclaw left a comment

Choose a reason for hiding this comment

rikkimax commented Nov 2, 2022

ibuclaw commented Nov 2, 2022

ibuclaw commented Nov 2, 2022 • edited Loading

rikkimax commented Nov 2, 2022

rikkimax commented Nov 3, 2022

RazvanN7 commented Nov 13, 2022

atilaneves commented Nov 15, 2022

rikkimax commented Nov 15, 2022 • edited Loading

rikkimax commented Dec 11, 2022

RazvanN7 commented Dec 11, 2022

rikkimax commented Dec 11, 2022

RazvanN7 commented Dec 15, 2022

dlang-bot commented Oct 31, 2022 •

edited

Loading

rikkimax commented Nov 1, 2022 •

edited

Loading

ibuclaw commented Nov 2, 2022 •

edited

Loading

rikkimax commented Nov 15, 2022 •

edited

Loading