Skip to content

Conversation

Al2Klimov
Copy link
Member

None of the derived classes use them, none shall have to explicitly delete them.

@Al2Klimov Al2Klimov requested a review from jschmidt-icinga May 22, 2025 14:18
@Al2Klimov Al2Klimov added the core/quality Improve code, libraries, algorithms, inline docs label May 22, 2025
@cla-bot cla-bot bot added the cla/signed label May 22, 2025
SharedObject(const SharedObject&) = delete;
SharedObject(SharedObject&&) = delete;
SharedObject& operator=(const SharedObject&) = delete;
SharedObject& operator=(SharedObject&&) = delete;
Copy link
Member Author

Choose a reason for hiding this comment

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

Wanted to do this for the new StoppableWaitGroup requested in #10397 (comment), but thought:

inline SharedObject& operator=(SharedObject&&)
{
return *this;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

what do they even exist for?

@Al2Klimov Al2Klimov changed the title SharedObject: delete unused methods Object: delete unused methods May 22, 2025
@Al2Klimov Al2Klimov changed the title Object: delete unused methods *Object: delete unused methods May 22, 2025
@jschmidt-icinga
Copy link
Contributor

You don't need to delete the move constructor and assignment operator when the copy constructor and assignment operator have already been deleted (or defined non-default).

See here:

If the definition of a class X does not explicitly declare a move constructor, a non-explicit one will be implicitly declared as defaulted if and only if

  • X does not have a user-declared copy constructor,
  • X does not have a user-declared copy assignment operator,
  • X does not have a user-declared move assignment operator, and
  • X does not have a user-declared destructor.

An equivalent rule exists for the operator, here.

So I agree absolutely with removing the definitions for shared object and defining the copy-constructor and copy-assignment operator as deleted, but for Object no change is needed.

@Al2Klimov Al2Klimov changed the title *Object: delete unused methods SharedObject: delete unused methods May 23, 2025
@Al2Klimov Al2Klimov force-pushed the SharedObject-delete branch from 6d77b3f to 352ef47 Compare May 23, 2025 08:36
Comment on lines 34 to 37
SharedObject(const SharedObject&) = delete;
SharedObject(SharedObject&&) = delete;
SharedObject& operator=(const SharedObject&) = delete;
SharedObject& operator=(SharedObject&&) = delete;
Copy link
Contributor

Choose a reason for hiding this comment

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

The move constructor and move-asignment operator deletes are still unnecessary.

None of the derived classes use them, none shall have to explicitly delete them.
@Al2Klimov Al2Klimov force-pushed the SharedObject-delete branch from 352ef47 to 4f351f6 Compare May 23, 2025 13:47
@Al2Klimov Al2Klimov merged commit 56d9f38 into master May 26, 2025
26 checks passed
@Al2Klimov Al2Klimov deleted the SharedObject-delete branch May 26, 2025 08:00
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed core/quality Improve code, libraries, algorithms, inline docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants