AK: Variant thing - forwarding [3/N] - #26547
Conversation
github-actions
Bot
added
the
👀 pr-needs-review
f0af688 to
4ff1fe0
Compare
|
Whoops pack indexing only works in gcc-15+ which ubuntu doesnt ship for 22.04, so it's off limits |
d32d9bc to
9ede9e3
Compare
|
Happy to have @alimpfard review this, but in case he's busy: From a distance, commit 3 looks nice and the tests in commit 2 are nice, but commit 2 is about the same amount of code lhs and rhs, so it feels a bit like a sideways change. |
alimpfard
left a comment
alimpfard
left a comment
There was a problem hiding this comment.
Largely LGTM, and I really like that the hacky constructors are gone (yay)
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
|
This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions! |
9ede9e3 to
0a78b88
Compare
0a78b88 to
d11a870
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
|
This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions! |
e124f21 to
8a5828f
Compare
f57855a to
0577d77
Compare
github-actions
Bot
added
the
👀 pr-needs-review
0577d77 to
3350c76
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
3350c76 to
653cd8b
Compare
|
Moved parts of it to #26914, to get some parts in faster |
github-actions
Bot
removed
the
👀 pr-needs-review
653cd8b to
67a5da6
Compare
Hendiadyoin1
changed the title
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! |
67a5da6 to
928dc85
Compare
Hendiadyoin1
added
⛔️ pr-is-blocked
This makes the overload logic a lot cleaner, as well as removing the concept of valid but unreachable variant states. Also this likely reduces some symbol sizes in LibSoftGPU, which to my knowledge was the only user of this feature. As a Side-effect the AK::Variant is now a type alias, so traits need to check against the `Detail` version.
For constructing the Storage we use a sentinel value (`VariantIndex`) to pass along the desired depth/type into the constructor chain. This is done as we cant easily start the lifetime of the alternatives after the union was instantiated. Upcoming c++26 features should fix this (see trivial unions (P3074R7) and std::start_lifetime (P3726R2)) For move and copy assignment we destroy the active member, and then replace the whole storage, trivial unions may also make this nicer. Also Overload resolution now returns the chosen overload wrapper, which now also holds the index of that overload, which avoids re-walking the type list to get the index.
928dc85 to
77dede9
Compare
github-actions
Bot
removed
the
⛔️ pr-is-blocked
77dede9 to
1596c4f
Compare
4 participants
While c++26 was not required for most of these, it did make it a bit easier at some points
This consists of 2 main parts:
This is step 1 of getting
constexprworking for it, and makes some more ugly parts of it disappearCC: @alimpfard as our local template and variant expert