-
-
Notifications
You must be signed in to change notification settings - Fork 411
Replace in with const scope for gc implementations
#2706
Conversation
|
Thanks for your pull request and interest in making D better, @JinShil! 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 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. 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 + druntime#2706" |
|
|
But that's not how the GC API is currently written. See... Lines 228 to 231 in a370406
Lines 238 to 241 in a370406
druntime/src/gc/impl/proto/gc.d Lines 27 to 28 in a370406
druntime/src/gc/impl/proto/gc.d Line 202 in a370406
druntime/src/gc/impl/proto/gc.d Line 207 in a370406
Should I modify |
But it used to have |
Would be nice to be more const-correct, but it might be rather viral as you'll have to change all GC implementations. Should be ok to leave the proxy function as is for now. |
|
I don't mind making all the changes. I'll try to change them to |
|
I tried to make everything const-correct, but as @rainers predicted it snowballed into too many changes outside the scope of this PR. So, I did only what was necessary to keep the status quo. It's clear there are some const-ness problems in the GC interface and implementations, but fixing that will have to be a different PR. |
|
Looks like core.gc.gcinterface.GC.runFinalizers() should be updated, too. |
Done. Thank you! |
PetarKirov
left a comment
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.
Thanks!
This PR also addresses #2704 (review)
cc @ZombineDev
About This PR
Followup to
#2676
dlang/phobos#7110
#2680
#2684
#2693
#2694
#2695
#2696
#2697
#2699
#2702
#2704
This one of several PRs I intend to submit, breaking up #2677 into multiple PRs.
This PR predominately addresses code in the gc implementations
Background
This PR is in support of dlang/dmd#10179
inas a parameter storage class is defined asscope const. Howeverinhas not yetbeen properly implemented so its current implementation is equivalent to
const. Properlyimplementing
innow will likely break code, so it is recommended to avoid usingin, andexplicitly use
constorscope constinstead, untilinis properly implemented.The use of
inas a parameter storage class is already discouraged in the documentation. See https://dlang.org/spec/function.html#parameters