Skip to content
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

Re-delete bitfield's copy-assignment operator #47

Merged
merged 3 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions common/BitField.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,23 @@ struct BitField
// Force default constructor to be created
// so that we can use this within unions
constexpr BitField() = default;
constexpr BitField(const BitField&) = default;
constexpr BitField(BitField&&) = default;

// Warning! This copies ALL storage bits, not just the bits represented by this bitfield.
// This exists so that unions can be copyable.
BitField& operator=(const BitField&) = default;
// We explicitly delete the copy assignment operator here, because the
// default copy assignment would copy the full storage value, rather than
// just the bits relevant to this particular bit field.
// Ideally, we would just implement the copy assignment to copy only the
// relevant bits, but we're prevented from doing that because the savestate
// code expects that this class is trivially copyable.
BitField& operator=(const BitField&) = delete;
// However, to allow assignment of unions returned by CGXDefault, we allow
// the use of the move assignment operator. This default implementation
// still copies all bits rather than just the bits relevant to this bit field,
// but this behavior is desirable when copying unions. Note that although
// the right-hand operand is not const (which is required for default to work),
// it is not modified.
constexpr BitField& operator=(BitField&&) = default;

BitField& operator=(T val)
{
Expand Down Expand Up @@ -229,6 +242,8 @@ struct BitFieldArray
// Force default constructor to be created
// so that we can use this within unions
constexpr BitFieldArray() = default;
constexpr BitFieldArray(const BitFieldArray&) = default;
constexpr BitFieldArray(BitFieldArray&&) = default;

// We explicitly delete the copy assignment operator here, because the
// default copy assignment would copy the full storage value, rather than
Expand All @@ -237,6 +252,13 @@ struct BitFieldArray
// relevant bits, but we're prevented from doing that because the savestate
// code expects that this class is trivially copyable.
BitFieldArray& operator=(const BitFieldArray&) = delete;
// However, to allow assignment of unions returned by CGXDefault, we allow
// the use of the move assignment operator. This default implementation
// still copies all bits rather than just the bits relevant to this bit field,
// but this behavior is desirable when copying unions. Note that although
// the right-hand operand is not const (which is required for default to work),
// it is not modified.
constexpr BitFieldArray& operator=(BitFieldArray&&) = default;

public:
constexpr bool IsSigned() const { return std::is_signed<T>(); }
Expand Down
6 changes: 3 additions & 3 deletions gxtest/cgx_defaults.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
#include "gxtest/XFMemory.h"

template <typename T>
static T CGXDefault();
static T CGXDefault() = delete;

template <typename T>
static T CGXDefault(int);
static T CGXDefault(int) = delete;

template <typename T>
static T CGXDefault(int, bool);
static T CGXDefault(int, bool) = delete;

template <>
GenMode CGXDefault<GenMode>()
Expand Down
2 changes: 1 addition & 1 deletion gxtest/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ Vec4<int> GetTevOutput(const GenMode& genmode, const TevStageCombiner::ColorComb
// Uses three additional TEV stages which shift the previous result
// three bits to the right. This is necessary to read off the 5 upper bits,
// 3 of which got masked off when writing to the EFB in the first pass.
gm = genmode;
gm.hex = genmode.hex;
gm.numtevstages = previous_stage + 3; // three additional stages
CGX_LOAD_BP_REG(gm.hex);

Expand Down