Re-delete bitfield's copy-assignment operator #47
Conversation
|
Note that this approach doesn't help with getting rid of the explicit copy constructors in RenderState.h in the main Dolphin repo, as they're used with const references. |
There was a problem hiding this comment.
Cool that the suggestion works so seamlessly!
I don't like this workaround in general, but considering it's just the hwtests repository, it's an acceptable trade-off. I wouldn't recommend porting this approach to Dolphin itself, so I'm relieved it doesn't trivially apply there :)
common/BitField.h
Outdated
| // code expects that this class is trivially copyable. | ||
| BitField& operator=(const BitField&) = delete; | ||
| // The move-assignment operator is still supplied so that unions can be | ||
| // assigned to. This still copies the full storage value, but that's the |
There was a problem hiding this comment.
Can you make this comment work in isolation (giving a reference to the copy assignment operator if needed)? Currently, it assumes the reader just finished reading the docstring of the copy-assignment operator. If that's not the case, "This still copies the full storage value" is confusing in that it doesn't highlight that this surprising behavior is a trap to watch out for.
common/BitField.h
Outdated
| // The move-assignment operator is still supplied so that unions can be | ||
| // assigned to. This still copies the full storage value, but that's the | ||
| // desired behavior when assigning to unions, so there's no issue with that. | ||
| // Note that this is still functions as a copy-assignment operator; the other |
There was a problem hiding this comment.
the other bitfield is not modified
Isn't it very much modified?
There was a problem hiding this comment.
Ah, I misread this comment. I suggest saying "right-hand operand" instead, as there's potential confusion with the other BitFields grouped in the current union.
This reverts commit 9b7be93. Note that this reversion means code no longer compiles properly. (Also note that BitField.h is still modified from the version used in Dolphin, to remove references to Inline.h.)
This allows assigning unions from CGXDefault, but still prevents the bad behavior of copying all bits when assigning one individual BitField to another.
Before, writing e.g. `CGXDefault<int>();` would compile successfully (with a warning), but would fail on linking. Now, it fails when compiling.
Pokechu22
force-pushed
the
bitfield-no-copy-assignment
branch
from
5c524b9 to
53c1707
Compare
Now, the move-assignment operator is provided so that unions can be assigned to with the CGX defaults.
See #46 (comment).