-
-
Notifications
You must be signed in to change notification settings - Fork 613
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 13392 and 11294 - class + alias this + cast(void*) == overzealous cast #9017
Conversation
Thanks for your pull request and interest in making D better, @RazvanN7! 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 + dmd#9017" |
|
You'll need to give this a kick to restart the doc tester. |
09710fe
to
2e6ba5b
Compare
Is it failing spuriously? I can't imagine how my patch would affect the documentation tester. |
Buildkite is failing. I have to investigate. |
dls looks like is using an older version of dub |
Any idea why |
2e6ba5b
to
6e65ca8
Compare
@thewilsonator I fixed those. It was an implementation problem. dls is still failing because it is not using the latest version of dub. @wilzbach Anything we can do about this, except from making a PR to change the dub version in dls? |
Thanks. |
I think we should wait until we fix dls before merging this, otherwise we will end up failing buildkite for the entire PR queue. |
|
@s-ludwig @MartinNowak @wilzbach what do you think about allowing out-of-band patch releases for dub? What I propose is to allow tagging new patch releases, so that projects using dub as a library could receive bug fixes without waiting for a new dmd release. The process should be as simple and fast as pushing a new tag to the dub repo. I don't think anything else would be needed. |
No real objection from my side, I've even done out-of-band patch releases a few times in the past. However, the change log process has changed recently, which may be a problem. The version file also needs to be updated in addition to the tag. |
Sure, we have done so before, but only as e.g.
If DLS has a |
|
@thewilsonator |
Yeah, see: d-language-server/dls#17.
Thanks! |
Restarted the dls build as a new version which selects dub |
All green! @thewilsonator |
Again op_overload mixes semantics. Before this patch when op_overload was called for a cast expression,
the result could mean 2 things: either an opCast exists or there is an alias this to be used for casting. 2 different semantics squashed in the same function call.