Don't return reference on copy assignment operators

We prefer to prevent using chained assignment (`T a = b = c = T();`) as this
can lead to confusing code and subtle bugs.

According to https://en.wikipedia.org/wiki/Assignment_operator_(C%2B%2B), C++
allows any arbitrary return type, so this is standard compliant.

This could be re-assessed if/when we have an actual need for a behavior more
akin to that of the C++ STL, for now this PR simply changes a handful of
cases which were inconsistent with the rest of the codebase (`void` return
type was already the most common case prior to this commit).
This commit is contained in:
Rémi Verschelde 2021-11-30 15:19:26 +01:00
parent 2d118bd8b8
commit 7da392bcc5
No known key found for this signature in database
GPG key ID: C3336907360768E1
28 changed files with 93 additions and 90 deletions

View file

@ -124,10 +124,9 @@ struct AudioFrame {
r = p_frame.r;
}
_ALWAYS_INLINE_ AudioFrame &operator=(const AudioFrame &p_frame) {
_ALWAYS_INLINE_ void operator=(const AudioFrame &p_frame) {
l = p_frame.l;
r = p_frame.r;
return *this;
}
_ALWAYS_INLINE_ operator Vector2() const {

View file

@ -134,12 +134,11 @@ public:
w(p_q.w) {
}
Quaternion &operator=(const Quaternion &p_q) {
void operator=(const Quaternion &p_q) {
x = p_q.x;
y = p_q.y;
z = p_q.z;
w = p_q.w;
return *this;
}
Quaternion(const Vector3 &v0, const Vector3 &v1) // shortest arc

View file

@ -130,9 +130,8 @@ Char16String &Char16String::operator+=(char16_t p_char) {
return *this;
}
Char16String &Char16String::operator=(const char16_t *p_cstr) {
void Char16String::operator=(const char16_t *p_cstr) {
copy_from(p_cstr);
return *this;
}
const char16_t *Char16String::get_data() const {
@ -186,9 +185,8 @@ CharString &CharString::operator+=(char p_char) {
return *this;
}
CharString &CharString::operator=(const char *p_cstr) {
void CharString::operator=(const char *p_cstr) {
copy_from(p_cstr);
return *this;
}
const char *CharString::get_data() const {

View file

@ -108,13 +108,10 @@ public:
_FORCE_INLINE_ Char16String() {}
_FORCE_INLINE_ Char16String(const Char16String &p_str) { _cowdata._ref(p_str._cowdata); }
_FORCE_INLINE_ Char16String &operator=(const Char16String &p_str) {
_cowdata._ref(p_str._cowdata);
return *this;
}
_FORCE_INLINE_ void operator=(const Char16String &p_str) { _cowdata._ref(p_str._cowdata); }
_FORCE_INLINE_ Char16String(const char16_t *p_cstr) { copy_from(p_cstr); }
Char16String &operator=(const char16_t *p_cstr);
void operator=(const char16_t *p_cstr);
bool operator<(const Char16String &p_right) const;
Char16String &operator+=(char16_t p_char);
int length() const { return size() ? size() - 1 : 0; }
@ -152,13 +149,10 @@ public:
_FORCE_INLINE_ CharString() {}
_FORCE_INLINE_ CharString(const CharString &p_str) { _cowdata._ref(p_str._cowdata); }
_FORCE_INLINE_ CharString &operator=(const CharString &p_str) {
_cowdata._ref(p_str._cowdata);
return *this;
}
_FORCE_INLINE_ void operator=(const CharString &p_str) { _cowdata._ref(p_str._cowdata); }
_FORCE_INLINE_ CharString(const char *p_cstr) { copy_from(p_cstr); }
CharString &operator=(const char *p_cstr);
void operator=(const char *p_cstr);
bool operator<(const CharString &p_right) const;
CharString &operator+=(char p_char);
int length() const { return size() ? size() - 1 : 0; }
@ -442,11 +436,7 @@ public:
_FORCE_INLINE_ String() {}
_FORCE_INLINE_ String(const String &p_str) { _cowdata._ref(p_str._cowdata); }
String &operator=(const String &p_str) {
_cowdata._ref(p_str._cowdata);
return *this;
}
_FORCE_INLINE_ void operator=(const String &p_str) { _cowdata._ref(p_str._cowdata); }
Vector<uint8_t> to_ascii_buffer() const;
Vector<uint8_t> to_utf8_buffer() const;

View file

@ -234,19 +234,17 @@ public:
data[i] = p_from.data[i];
}
}
inline LocalVector &operator=(const LocalVector &p_from) {
inline void operator=(const LocalVector &p_from) {
resize(p_from.size());
for (U i = 0; i < p_from.count; i++) {
data[i] = p_from.data[i];
}
return *this;
}
inline LocalVector &operator=(const Vector<T> &p_from) {
inline void operator=(const Vector<T> &p_from) {
resize(p_from.size());
for (U i = 0; i < count; i++) {
data[i] = p_from[i];
}
return *this;
}
_FORCE_INLINE_ ~LocalVector() {

View file

@ -353,7 +353,7 @@ public:
(*this) = p_other;
}
OAHashMap &operator=(const OAHashMap &p_other) {
void operator=(const OAHashMap &p_other) {
if (capacity != 0) {
clear();
}
@ -363,7 +363,6 @@ public:
for (Iterator it = p_other.iter(); it.valid; it = p_other.next_iter(it)) {
set(*it.key, *it.value);
}
return *this;
}
OAHashMap(uint32_t p_initial_capacity = 64) {

View file

@ -85,11 +85,10 @@ public:
next_element(other.next_element) {
}
Element &operator=(const Element &other) {
void operator=(const Element &other) {
list_element = other.list_element;
next_element = other.next_element;
prev_element = other.prev_element;
return *this;
}
_FORCE_INLINE_ bool operator==(const Element &p_other) const {
@ -145,9 +144,8 @@ public:
list_element(other.list_element) {
}
ConstElement &operator=(const ConstElement &other) {
void operator=(const ConstElement &other) {
list_element = other.list_element;
return *this;
}
ConstElement next() const {

View file

@ -132,9 +132,8 @@ public:
insert(i, p_val);
}
inline Vector &operator=(const Vector &p_from) {
inline void operator=(const Vector &p_from) {
_cowdata._ref(p_from._cowdata);
return *this;
}
Vector<uint8_t> to_byte_array() const {

View file

@ -193,9 +193,8 @@ public:
_FORCE_INLINE_ VMap() {}
_FORCE_INLINE_ VMap(const VMap &p_from) { _cowdata._ref(p_from._cowdata); }
inline VMap &operator=(const VMap &p_from) {
inline void operator=(const VMap &p_from) {
_cowdata._ref(p_from._cowdata);
return *this;
}
};

View file

@ -66,14 +66,13 @@ class AudioStreamPreviewGenerator : public Node {
Thread *thread = nullptr;
// Needed for the bookkeeping of the Map
Preview &operator=(const Preview &p_rhs) {
void operator=(const Preview &p_rhs) {
preview = p_rhs.preview;
base_stream = p_rhs.base_stream;
playback = p_rhs.playback;
generating.set_to(generating.is_set());
id = p_rhs.id;
thread = p_rhs.thread;
return *this;
}
Preview(const Preview &p_rhs) {
preview = p_rhs.preview;

View file

@ -192,10 +192,12 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::VECTOR2I: {
int id = variable_id++;
Vector2 vec = p_var;
const String type_scalar = Variant::get_type_name(p_var.get_type() == Variant::VECTOR2 ? Variant::FLOAT : Variant::INT);
DAP::Variable x, y;
x.name = "x";
y.name = "y";
x.type = y.type = Variant::get_type_name(p_var.get_type() == Variant::VECTOR2 ? Variant::FLOAT : Variant::INT);
x.type = type_scalar;
y.type = type_scalar;
x.value = rtos(vec.x);
y.value = rtos(vec.y);
@ -209,12 +211,16 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::RECT2I: {
int id = variable_id++;
Rect2 rect = p_var;
const String type_scalar = Variant::get_type_name(p_var.get_type() == Variant::RECT2 ? Variant::FLOAT : Variant::INT);
DAP::Variable x, y, w, h;
x.name = "x";
y.name = "y";
w.name = "w";
h.name = "h";
x.type = y.type = w.type = h.type = Variant::get_type_name(p_var.get_type() == Variant::RECT2 ? Variant::FLOAT : Variant::INT);
x.type = type_scalar;
y.type = type_scalar;
w.type = type_scalar;
h.type = type_scalar;
x.value = rtos(rect.position.x);
y.value = rtos(rect.position.y);
w.value = rtos(rect.size.x);
@ -232,11 +238,14 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::VECTOR3I: {
int id = variable_id++;
Vector3 vec = p_var;
const String type_scalar = Variant::get_type_name(p_var.get_type() == Variant::VECTOR3 ? Variant::FLOAT : Variant::INT);
DAP::Variable x, y, z;
x.name = "x";
y.name = "y";
z.name = "z";
x.type = y.type = z.type = Variant::get_type_name(p_var.get_type() == Variant::VECTOR3 ? Variant::FLOAT : Variant::INT);
x.type = type_scalar;
y.type = type_scalar;
z.type = type_scalar;
x.value = rtos(vec.x);
y.value = rtos(vec.y);
z.value = rtos(vec.z);
@ -251,11 +260,14 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::TRANSFORM2D: {
int id = variable_id++;
Transform2D transform = p_var;
const String type_vec2 = Variant::get_type_name(Variant::VECTOR2);
DAP::Variable x, y, origin;
x.name = "x";
y.name = "y";
origin.name = "origin";
x.type = y.type = origin.type = Variant::get_type_name(Variant::VECTOR2);
x.type = type_vec2;
y.type = type_vec2;
origin.type = type_vec2;
x.value = transform.elements[0];
y.value = transform.elements[1];
origin.value = transform.elements[2];
@ -291,12 +303,16 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::QUATERNION: {
int id = variable_id++;
Quaternion quat = p_var;
const String type_float = Variant::get_type_name(Variant::FLOAT);
DAP::Variable x, y, z, w;
x.name = "x";
y.name = "y";
z.name = "z";
w.name = "w";
x.type = y.type = z.type = w.type = Variant::get_type_name(Variant::FLOAT);
x.type = type_float;
y.type = type_float;
z.type = type_float;
w.type = type_float;
x.value = rtos(quat.x);
y.value = rtos(quat.y);
z.value = rtos(quat.z);
@ -313,10 +329,12 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::AABB: {
int id = variable_id++;
AABB aabb = p_var;
const String type_vec3 = Variant::get_type_name(Variant::VECTOR3);
DAP::Variable position, size;
position.name = "position";
size.name = "size";
position.type = size.type = Variant::get_type_name(Variant::VECTOR3);
position.type = type_vec3;
size.type = type_vec3;
position.value = aabb.position;
size.value = aabb.size;
position.variablesReference = parse_variant(aabb.position);
@ -331,11 +349,14 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::BASIS: {
int id = variable_id++;
Basis basis = p_var;
const String type_vec2 = Variant::get_type_name(Variant::VECTOR2);
DAP::Variable x, y, z;
x.name = "x";
y.name = "y";
z.name = "z";
x.type = y.type = z.type = Variant::get_type_name(Variant::VECTOR2);
x.type = type_vec2;
y.type = type_vec2;
z.type = type_vec2;
x.value = basis.elements[0];
y.value = basis.elements[1];
z.value = basis.elements[2];
@ -372,12 +393,16 @@ int DebugAdapterProtocol::parse_variant(const Variant &p_var) {
case Variant::COLOR: {
int id = variable_id++;
Color color = p_var;
const String type_float = Variant::get_type_name(Variant::FLOAT);
DAP::Variable r, g, b, a;
r.name = "r";
g.name = "g";
b.name = "b";
a.name = "a";
r.type = g.type = b.type = a.type = Variant::get_type_name(Variant::FLOAT);
r.type = type_float;
g.type = type_float;
b.type = type_float;
a.type = type_float;
r.value = rtos(color.r);
g.value = rtos(color.g);
b.value = rtos(color.b);

View file

@ -230,11 +230,10 @@ private:
render_db_value = n.render_db_value;
}
_FORCE_INLINE_ AudioNotch &operator=(const EditorAudioMeterNotches::AudioNotch &n) {
_FORCE_INLINE_ void operator=(const EditorAudioMeterNotches::AudioNotch &n) {
relative_position = n.relative_position;
db_value = n.db_value;
render_db_value = n.render_db_value;
return *this;
}
_FORCE_INLINE_ AudioNotch() {}

View file

@ -488,8 +488,8 @@ void EditorExportPlatform::_edit_files_with_filter(DirAccess *da, const Vector<S
String cur_dir_no_prefix = cur_dir.replace("res://", "");
Vector<String> dirs;
String f;
while ((f = da->get_next()) != "") {
String f = da->get_next();
while (!f.is_empty()) {
if (da->current_is_dir()) {
dirs.push_back(f);
} else {
@ -506,6 +506,7 @@ void EditorExportPlatform::_edit_files_with_filter(DirAccess *da, const Vector<S
}
}
}
f = da->get_next();
}
da->list_dir_end();

View file

@ -761,10 +761,11 @@ void EditorFileDialog::update_file_list() {
List<String> files;
List<String> dirs;
String item;
String item = dir_access->get_next();
while ((item = dir_access->get_next()) != "") {
while (!item.is_empty()) {
if (item == "." || item == "..") {
item = dir_access->get_next();
continue;
}
@ -775,6 +776,7 @@ void EditorFileDialog::update_file_list() {
dirs.push_back(item);
}
}
item = dir_access->get_next();
}
dirs.sort_custom<NaturalNoCaseComparator>();

View file

@ -192,7 +192,7 @@ public:
GDScriptDataType() = default;
GDScriptDataType &operator=(const GDScriptDataType &p_other) {
void operator=(const GDScriptDataType &p_other) {
kind = p_other.kind;
has_type = p_other.has_type;
builtin_type = p_other.builtin_type;
@ -203,7 +203,6 @@ public:
if (p_other.has_container_element_type()) {
set_container_element_type(p_other.get_container_element_type());
}
return *this;
}
GDScriptDataType(const GDScriptDataType &p_other) {

View file

@ -203,7 +203,7 @@ public:
return !(this->operator==(p_other));
}
DataType &operator=(const DataType &p_other) {
void operator=(const DataType &p_other) {
kind = p_other.kind;
type_source = p_other.type_source;
is_constant = p_other.is_constant;
@ -221,7 +221,6 @@ public:
if (p_other.has_container_element_type()) {
set_container_element_type(p_other.get_container_element_type());
}
return *this;
}
DataType() = default;

View file

@ -598,7 +598,7 @@ class BindingsGenerator {
private:
NameCache(const NameCache &);
NameCache &operator=(const NameCache &);
void operator=(const NameCache &);
};
NameCache name_cache;

View file

@ -229,9 +229,6 @@ private:
#endif
}
_GodotSharpDirs(const _GodotSharpDirs &);
_GodotSharpDirs &operator=(const _GodotSharpDirs &);
public:
static _GodotSharpDirs &get_singleton() {
static _GodotSharpDirs singleton;

View file

@ -56,13 +56,12 @@ struct MonoGCHandleData {
void release();
MonoGCHandleData &operator=(const MonoGCHandleData &p_other) {
void operator=(const MonoGCHandleData &p_other) {
#ifdef DEBUG_ENABLED
CRASH_COND(!is_released());
#endif
handle = p_other.handle;
type = p_other.type;
return *this;
}
MonoGCHandleData(const MonoGCHandleData &) = default;

View file

@ -121,12 +121,10 @@ void GDMonoLog::_delete_old_log_files(const String &p_logs_dir) {
ERR_FAIL_COND(da->list_dir_begin() != OK);
String current;
while ((current = da->get_next()).length()) {
if (da->current_is_dir()) {
continue;
}
if (!current.ends_with(".txt")) {
String current = da->get_next();
while (!current.is_empty()) {
if (da->current_is_dir() || !current.ends_with(".txt")) {
current = da->get_next();
continue;
}
@ -135,6 +133,7 @@ void GDMonoLog::_delete_old_log_files(const String &p_logs_dir) {
if (OS::get_singleton()->get_unix_time() - modified_time > MAX_SECS) {
da->remove(current);
}
current = da->get_next();
}
da->list_dir_end();

View file

@ -728,10 +728,10 @@ Error EditorExportPlatformIOS::_export_loading_screen_images(const Ref<EditorExp
Error EditorExportPlatformIOS::_walk_dir_recursive(DirAccess *p_da, FileHandler p_handler, void *p_userdata) {
Vector<String> dirs;
String path;
String current_dir = p_da->get_current_dir();
p_da->list_dir_begin();
while ((path = p_da->get_next()).length() != 0) {
String path = p_da->get_next();
while (!path.is_empty()) {
if (p_da->current_is_dir()) {
if (path != "." && path != "..") {
dirs.push_back(path);
@ -743,6 +743,7 @@ Error EditorExportPlatformIOS::_walk_dir_recursive(DirAccess *p_da, FileHandler
return err;
}
}
path = p_da->get_next();
}
p_da->list_dir_end();

View file

@ -960,9 +960,10 @@ void EditorExportPlatformOSX::_zip_folder_recursive(zipFile &p_zip, const String
DirAccessRef da = DirAccess::open(dir);
da->list_dir_begin();
String f;
while ((f = da->get_next()) != "") {
String f = da->get_next();
while (!f.is_empty()) {
if (f == "." || f == "..") {
f = da->get_next();
continue;
}
if (da->is_link(f)) {
@ -1065,6 +1066,7 @@ void EditorExportPlatformOSX::_zip_folder_recursive(zipFile &p_zip, const String
zipCloseFileInZip(p_zip);
}
f = da->get_next();
}
da->list_dir_end();
}

View file

@ -100,12 +100,11 @@ SoftDynamicBody3D::PinnedPoint::PinnedPoint(const PinnedPoint &obj_tocopy) {
offset = obj_tocopy.offset;
}
SoftDynamicBody3D::PinnedPoint &SoftDynamicBody3D::PinnedPoint::operator=(const PinnedPoint &obj) {
void SoftDynamicBody3D::PinnedPoint::operator=(const PinnedPoint &obj) {
point_index = obj.point_index;
spatial_attachment_path = obj.spatial_attachment_path;
spatial_attachment = obj.spatial_attachment;
offset = obj.offset;
return *this;
}
void SoftDynamicBody3D::_update_pickable() {

View file

@ -80,7 +80,7 @@ public:
PinnedPoint();
PinnedPoint(const PinnedPoint &obj_tocopy);
PinnedPoint &operator=(const PinnedPoint &obj);
void operator=(const PinnedPoint &obj);
};
private:

View file

@ -473,6 +473,13 @@ void FileDialog::update_file_list() {
dir_access->list_dir_begin();
if (dir_access->is_readable(dir_access->get_current_dir().utf8().get_data())) {
message->hide();
} else {
message->set_text(TTRC("You don't have permission to access contents of this folder."));
message->show();
}
TreeItem *root = tree->create_item();
Ref<Texture2D> folder = vbox->get_theme_icon(SNAME("folder"), SNAME("FileDialog"));
Ref<Texture2D> file_icon = vbox->get_theme_icon(SNAME("file"), SNAME("FileDialog"));
@ -482,17 +489,11 @@ void FileDialog::update_file_list() {
List<String> dirs;
bool is_hidden;
String item;
String item = dir_access->get_next();
if (dir_access->is_readable(dir_access->get_current_dir().utf8().get_data())) {
message->hide();
} else {
message->set_text(TTRC("You don't have permission to access contents of this folder."));
message->show();
}
while ((item = dir_access->get_next()) != "") {
while (!item.is_empty()) {
if (item == "." || item == "..") {
item = dir_access->get_next();
continue;
}
@ -505,6 +506,7 @@ void FileDialog::update_file_list() {
dirs.push_back(item);
}
}
item = dir_access->get_next();
}
dirs.sort_custom<NaturalNoCaseComparator>();

View file

@ -758,10 +758,9 @@ class PhysicsServer2DManager {
name(p_ci.name),
create_callback(p_ci.create_callback) {}
ClassInfo &operator=(const ClassInfo &p_ci) {
void operator=(const ClassInfo &p_ci) {
name = p_ci.name;
create_callback = p_ci.create_callback;
return *this;
}
};

View file

@ -965,10 +965,9 @@ class PhysicsServer3DManager {
name(p_ci.name),
create_callback(p_ci.create_callback) {}
ClassInfo &operator=(const ClassInfo &p_ci) {
void operator=(const ClassInfo &p_ci) {
name = p_ci.name;
create_callback = p_ci.create_callback;
return *this;
}
};

View file

@ -55,7 +55,10 @@ struct CountedItem {
count++;
}
CountedItem &operator=(const CountedItem &p_other) = default;
void operator=(const CountedItem &p_other) {
id = p_other.id;
count++;
}
~CountedItem() {
CRASH_COND(destroyed);