-
-
Notifications
You must be signed in to change notification settings - Fork 701
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
Fix issue 4957 - std.concurrency rejects structs with Tid #6738
Conversation
Geod24
commented
Oct 22, 2018
I hope this gets merged, this and https://issues.dlang.org/show_bug.cgi?id=5570 made me feel that Dlang was not stable enough a few years back. I've seen more activity in these bug reports these days, I'm feeling tempted to try Dlang again ☕️ . |
cd1ee3a
to
a525c7e
Compare
Thanks for your pull request and interest in making D better, @Geod24! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
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
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + phobos#6738" |
But doesn't the old code short-circuit, and yours does not? Then again maybe the performance is ultimately better because of a lack of recursive template instantiations.. ¯\_(ツ)_/¯ |
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.
Including short-circuiting would be nice, but I'll approve this even without.
43d4202
to
d59ade8
Compare
Is there a way to short-circuit without recursive instantiation ? |
In my experience you can write |
Don't think this has much influence, because by the time the code is run at CTFE all we have is |
For the benefit of dlang-bot I edited the commit message to start "Fix Issue 4957" instead of "Fix 4957", but that's been clobbered. Please make that change. |
I did already, but dlang-bot doesn't seem to realize this. |
In addition to the fix which makes it recurse in struct, the instantiation was changed to not use recursive template instantiation, exception for struct's tupleof.
@n8sh : Ah, commit message, not PR title. Sorry. Thanks for the fix! |
Yep that's needed to aggregate the changelog entries over commit messages |