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.

Sign up for GitHub

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

Fixed -Wdeprecated-copy-dtor warnings by implementing a copy assignme… #3306

Merged
merged 1 commit into from
Mar 9, 2024
Merged

Fixed -Wdeprecated-copy-dtor warnings by implementing a copy assignme… #3306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions c++/src/H5Attribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,4 +605,16 @@ Attribute::~Attribute()
}
}

//--------------------------------------------------------------------------
// Function: Copy assignment operator
Attribute &
Attribute::operator=(const Attribute &original)
{
if (&original != this) {
setId(original.id);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setId(original.id); needed here so that both objects have the same hdf5 ID (see DataSet, DataType,...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. This is the crux of the WIP. I'm not sure what to actually put into these copy assignment operators...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that a copy in this context is the equivalent of (re-)opening the thing in the same way, so you'd want a new, different ID. Unfortunately, we don't have a lightweight way of duplicating an ID, though that could be added pretty easily. There are calls that do what the library would do (e.g., H5Iobject_verify() and H5Iregister()), but we don't allow using those with library ID types (just user-defined ID types).

Copy link
Contributor

@bmribler bmribler Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the HDF5 C++ API, the operator= uses the same ID as the original because if a different ID is created, it won't have the same characteristics as the original ID, specifically, if the rhs represents a named datatype, "this" would still be a transient datatype. At least, that was my understanding at the time. However, I just realized PropList didn't follow that concept but I don't remember why and I don't know if it needs to be fixed... I'll have to look into that.


return *this;
}

} // namespace H5
3 changes: 3 additions & 0 deletions c++/src/H5Attribute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ class H5_DLLCPP Attribute : public AbstractDs, public H5Location {
// Destructor: properly terminates access to this attribute.
virtual ~Attribute() override;

// Copy assignment operator.
Attribute &operator=(const Attribute &original);

#ifndef DOXYGEN_SHOULD_SKIP_THIS
protected:
// Sets the attribute id.
Expand Down
12 changes: 12 additions & 0 deletions c++/src/H5Group.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,4 +274,16 @@ Group::~Group()
}
}

//--------------------------------------------------------------------------
// Function: Copy assignment operator
Group &
Group::operator=(const Group &original)
{
if (&original != this) {
setId(original.id);
}
seanm marked this conversation as resolved.
Show resolved Hide resolved

return *this;
}

} // namespace H5
3 changes: 3 additions & 0 deletions c++/src/H5Group.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ class H5_DLLCPP Group : public H5Object, public CommonFG {
// Destructor
virtual ~Group() override;

// Copy assignment operator.
Group &operator=(const Group &original);

// Creates a copy of an existing group using its id.
Group(const hid_t group_id);

Expand Down
Loading