-
-
Notifications
You must be signed in to change notification settings - Fork 747
Add public API for regex charset parser #5337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ccbbc17 to
0dd4244
Compare
|
@DmitryOlshansky generated/d_do_test.o(.text._D3std4conv29__T8textImplTAyaTAyaTkTAyaTkZ8textImplFNaNbNfAyakAyakZAya+0x3d): In function `_D3std4conv29__T8textImplTAyaTAyaTkTAyaTkZ8textImplFNaNbNfAyakAyakZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std4conv29__T8textImplTAyaTAyaTkTAyaTkZ8textImplFNaNbNfAyakAyakZAya+0x79): In function `_D3std4conv29__T8textImplTAyaTAyaTkTAyaTkZ8textImplFNaNbNfAyakAyakZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std5array17__T8AppenderTAyaZ8Appender94__T3putTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ3putMFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZv+0x38): In function `_D3std5array17__T8AppenderTAyaZ8Appender94__T3putTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ3putMFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZv':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result5emptyMFNaNbNdNiNfZb'
generated/d_do_test.o(.text._D3std5array17__T8AppenderTAyaZ8Appender94__T3putTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ3putMFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZv+0x4a): In function `_D3std5array17__T8AppenderTAyaZ8Appender94__T3putTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ3putMFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZv':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result5frontMFNaNbNdNiNfZa'
generated/d_do_test.o(.text._D3std5array17__T8AppenderTAyaZ8Appender94__T3putTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ3putMFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZv+0x5b): In function `_D3std5array17__T8AppenderTAyaZ8Appender94__T3putTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ3putMFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZv':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result8popFrontMFNaNbNiNfZv'
generated/d_do_test.o(.text._D3std4conv17__T6toImplTAyaTkZ6toImplFNaNbNekkE3std5ascii10LetterCaseZAya+0x51): In function `_D3std4conv17__T6toImplTAyaTkZ6toImplFNaNbNekkE3std5ascii10LetterCaseZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa+0x10): In function `_D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result6lengthMFNaNbNdNiNfZk'
generated/d_do_test.o(.text._D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa+0x4b): In function `_D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result5emptyMFNaNbNdNiNfZb'
generated/d_do_test.o(.text._D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa+0x57): In function `_D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result5frontMFNaNbNdNiNfZa'
generated/d_do_test.o(.text._D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa+0x8c): In function `_D3std5array96__T5arrayTS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZ5arrayFNaNbNfS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6ResultZAa':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result8popFrontMFNaNbNiNfZv'
generated/d_do_test.o(.text._D3std4conv36__T8textImplTAyaTAyaTkTAyaTykTAyaTkZ8textImplFNaNbNfAyakAyaykAyakZAya+0x3d): In function `_D3std4conv36__T8textImplTAyaTAyaTkTAyaTykTAyaTkZ8textImplFNaNbNfAyakAyaykAyakZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std4conv36__T8textImplTAyaTAyaTkTAyaTykTAyaTkZ8textImplFNaNbNfAyakAyaykAyakZAya+0xb5): In function `_D3std4conv36__T8textImplTAyaTAyaTkTAyaTykTAyaTkZ8textImplFNaNbNfAyakAyaykAyakZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std4conv30__T8textImplTAyaTAyaTkTAyaTykZ8textImplFNaNbNfAyakAyaykZAya+0x3d): In function `_D3std4conv30__T8textImplTAyaTAyaTkTAyaTykZ8textImplFNaNbNfAyakAyaykZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std4conv23__T8textImplTAyaTAyaTkZ8textImplFNaNbNfAyakZAya+0x3d): In function `_D3std4conv23__T8textImplTAyaTAyaTkZ8textImplFNaNbNfAyakZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
generated/d_do_test.o(.text._D3std4conv27__T8textImplTAyaTAyaTkTAyaZ8textImplFNaNbNfAyakAyaZAya+0x3a): In function `_D3std4conv27__T8textImplTAyaTAyaTkTAyaZ8textImplFNaNbNfAyakAyaZAya':
: undefined reference to `_D3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZS3std4conv47__T7toCharsVhi10TaVE3std5ascii10LetterCasei1TkZ7toCharsFNaNbNiNfkZ6Result'
Error: linker exited with status 1 |
0dd4244 to
680c07c
Compare
|
Let's hope it likes it now. |
|
|
This has been green for awhile now. How should we move forward with this? |
| return val; | ||
| } | ||
|
|
||
| @system unittest //BUG canFind is system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it's being moved, I think it's worth noting that canFind is no longer system (see its @safe unittest), so this can be changed.
From my side, absoultely - |
std/uni.d
Outdated
| import std.traits; // isConvertibleToString, isIntegral, isSomeChar, | ||
| // isSomeString, Unqual | ||
|
|
||
| import std.exception; // enforce |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use selective imports to avoid symbol leakage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last time I checked it was the other way around
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but then we discovered that non-selective imports are leaking:
tl;dr: if you add std.exception here, people can import all symbols from std.exception from std.uni
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wilzbach according to the linked comment even selective imports leak so it wouldn't help to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh for **ck sake... I'll change this one but the other will stay as is for now, I don't want to mix concerns in a single pull request that supposed to be refactoring.
std/uni.d
Outdated
| /// | ||
| @safe unittest | ||
| { | ||
| import std.uni; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI error is:
Local imports should specify the symbols being imported to avoid hiding local symbols.
std/uni.d
Outdated
| static package CodepointSet parsePropertySpec(Range)(ref Range p, | ||
| bool negated, bool casefold) | ||
| { | ||
| import std.ascii; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI error
Local imports should specify the symbols being imported to avoid hiding local symbols.
This only does the move without adapting the interface to the commonly accepted ranges.
680c07c to
bc14c02
Compare
|
Thanks for your pull request, @DmitryOlshansky! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
|
The only other concern is the test coverage (29% seems pretty poor), but I don't know if that's worth delaying merging. |
I'm tired to hell of explaining that our FREAKING AMAZING CI will only do a FILE AT A TIME TEST to show coverage. Regex tests live in a separate file, no way a file at a time will catch proper coverage stats. TL;DR: Our coverage report is horribly broken, not my problem though. |
|
@DmitryOlshansky fair enough. I'll leave it to @wilzbach to merge. |
Yup.
Yes :/ |
|
This PR caused a -dip1000 regression: https://issues.dlang.org/show_bug.cgi?id=17961 tl;dr: Now it's impossible to compile any D code with |
|
This PR also caused a regression in compilation times: https://issues.dlang.org/show_bug.cgi?id=18378 :-( |
|
See #5981 |
|
I'm talking about compilation speed, not the performance of the regex code itself. Both need to be fixed. :-P |
It's a bunch of unwieldy code that I planed to reuse long ago.
Libraries such as ICU also do provide means to construct sets of codepoints from strings with regex syntax.