Skip to content

Fix issue #22535 - [REG 2.112] Unicode symbols inside q#22539

Merged
thewilsonator merged 1 commit intodlang:stablefrom
rikkimax:fix-issue-22535
Feb 20, 2026
Merged

Fix issue #22535 - [REG 2.112] Unicode symbols inside q#22539
thewilsonator merged 1 commit intodlang:stablefrom
rikkimax:fix-issue-22535

Conversation

@rikkimax
Copy link
Contributor

@rikkimax rikkimax commented Feb 9, 2026

No description provided.

@dlang-bot
Copy link
Contributor

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.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

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 "stable + dmd#22539"

Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test case for this?

@ibuclaw
Copy link
Member

ibuclaw commented Feb 9, 2026

IIUC

assert(q"EOS
是的0
EOS" == "是的0");

Or one can cast to int[] and assert each index has the correct numerical value.

@rikkimax rikkimax force-pushed the fix-issue-22535 branch 2 times, most recently from 2450675 to 4331583 Compare February 15, 2026 11:05
@rikkimax
Copy link
Contributor Author

Guess I haven't improved stable urgh.

@rikkimax
Copy link
Contributor Author

Swapping the string buffers from identifier getting its own, to a secondary one for q strings

@rikkimax
Copy link
Contributor Author

Okay I'm getting rather sick of this.

 ... dshell/cpp_header_gen.d
 !!! dshell/cpp_header_gen.d is disabled!
 ... dshell/defaults.d
 ... dshell/depsprot.d
 ... dshell/dll.d
 !!! dshell/dll.d is disabled!
 ... dshell/dll_cxx.d
 !!! dshell/dll_cxx.d is disabled!
 ... dshell/dwarf.d
 !!! dshell/dwarf.d is disabled!
 ... dshell/issue20444_SOURCE_DATE_EPOCH.d
 ... dshell/issue22804.d
 ... dshell/issue22816.d
 ... dshell/issue22817.d
 ... dshell/issue24111.d
 ... dshell/linker_flag_with_spaces.d
 ... dshell/sameenv.d
 ... dshell/test6952.d
 !!! dshell/test6952.d is disabled!
 ... dshell/test9377.d
 ... dshell/test_shared.d
 !!! dshell/test_shared.d is disabled!

From what I can tell, the dshell tests should've completed, and had a good 10m+ left, but the phobos ones don't start.

@rainers any ideas?

I've backported a bunch of PR's at this point and tried to hunt down more.

class Lexer
{
private __gshared OutBuffer stringbuffer;
private __gshared
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

silly question: why are these __gshared in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who knows, it'll be faster than TLS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be surprised if there was a meaningful difference in performance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean why are they not a regular member of the lexer? We only have one instance of the lexer at one time anyway, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thewilsonator thread-local variables may be slightly slower than __gshared due to additional pointer dereference (ARM and RISC-V isn't affected by this, but x86 - maybe)
I guess it was important at one time

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler is single threaded, no need for tls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibuclaw seems, it will still give a performance penalty, only if the key like --fthread-model=initial-exec is not used for the compiler build

(I think these are all minor details, and for code readability and to avoid errors it's worth simply removing __gshared from here)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler is single threaded, no need for tls.

What about dmd-as-a-library?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are lot of places where globals are used in the frontend.

An audit would need to take place, and a discussion with Walter.

I won't be changing this in this PR, this isn't the right PR for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler is single threaded, no need for tls.

What about dmd-as-a-library?

GDC and LDC use dmd as a library.

Consider that mingw and darwin are emutls targets as well.

@rikkimax rikkimax force-pushed the fix-issue-22535 branch 3 times, most recently from a7b2f49 to 15d3809 Compare February 19, 2026 10:57
@rikkimax
Copy link
Contributor Author

Okay dshell tests are passing, so its something to do with either the run.d, phobos~stable, ldc, or visuald.

Will try bumping ldc and visuald first.

@rikkimax
Copy link
Contributor Author

Ah huh!

Azure didn't tell me what failed the CI: runnable\lexer.d

Joy.

@rikkimax
Copy link
Contributor Author

FOUND IT

FOUND IT

FOUND IT

A VARIANT OF LIBC isalpha CANNOT ACCEPT UNICODE >= 0x80

SWEAR WORDS GO HERE

@rikkimax rikkimax force-pushed the fix-issue-22535 branch 2 times, most recently from 77539e8 to aa94cca Compare February 19, 2026 15:02
@rikkimax
Copy link
Contributor Author

The behavior of isalpha and _isalpha_l is undefined if c isn't EOF or in the range 0 through 0xFF, inclusive. When a debug CRT library is used and c isn't one of these values, the functions raise an assertion.

@rikkimax
Copy link
Contributor Author

And its green, finally.

@thewilsonator thewilsonator merged commit f2c0a8c into dlang:stable Feb 20, 2026
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants