Added bitfield support for std.typecons.Flag and other boolean types#5441
Added bitfield support for std.typecons.Flag and other boolean types#5441marler8997 wants to merge 1 commit intodlang:masterfrom
Conversation
marler8997
force-pushed
the
AddBitFieldFlags
branch
from
b942b0d to
115f1db
Compare
|
I just remembered I had submitted this PR. What's the Approver workflow here? Is there a process that guarantees someone will eventually look at this or could it get lost in time if no one ever reviews it? |
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
What's the Approver workflow here? Is there a process that guarantees someone will eventually look at this or could it get lost in time if no one ever reviews it?
There's no fixed process. While @MartinNowak and I are working to hook up dlang-bot, s.t. it can detect stalled PRs without review, there's simply a lack of reviewers atm.
However, pinging your PR if it went undetected definitely helps in the meantime.
In general I have to say that I think that this is a very cool addition and makes bitfields even more powerful :)
I have added a couple of small nits and could you please add a short changelog file?
https://github.com/dlang/phobos/tree/master/changelog
It helps a lot with approval, because then it can be immediately pulled.
std/bitmanip.d
Outdated
| { | ||
| static if(__traits(compiles, TemplateOf!T)) | ||
| { | ||
| enum TypeName = T.stringof~"!"~TemplateArgsOf!T.stringof[5..$]; |
There was a problem hiding this comment.
Nit: D Style is to have a space between binary operators
std/bitmanip.d
Outdated
| t.c = Yes.CustomFlag2; | ||
| t.d = CustomFlag3!"something".true_; | ||
| assert(t.a == Yes.a); | ||
| assert(t.b == CustomFlag1.second); |
There was a problem hiding this comment.
I would also test whether setting/inverting the bits works as expected.
| { | ||
| enum TypeName = T.stringof; | ||
| } | ||
| } |
There was a problem hiding this comment.
Hmm, I was just looking at std.traits for five minutes, because I was convinced that something such general like this already existed, but apparently it doesn't.
There was a problem hiding this comment.
It doesn't exist because it doesn't work in the general case. Making it work in the general case is quite hard, I have a few idea on it. It would be a good addition to phobos but there's quite a few things to discuss on it. Making it work well may also require additions to the language.
marler8997
force-pushed
the
AddBitFieldFlags
branch
5 times, most recently
from
1a8658e to
5d415de
Compare
marler8997
force-pushed
the
AddBitFieldFlags
branch
from
5d415de to
d220c94
Compare
|
NOTE: This PR is less relevant if #5490 gets accepted. Both could still be integrated but if bitfields2 is accepted I would think bitfields should be deprecated so enhancing it may actually be counter-productive. |
|
Cleaning up old PRs that aren't going anywhere |
3 participants
No description provided.