Skip to content

modernize-use-equals-delete should recommend relocating only special members to publicΒ #135249

Open
@duncanthomson

Description

@duncanthomson

From the doc as of v21.0.0), modernize-use-equals-delete:

Identifies unimplemented private special member functions, and recommends using = delete for them.

This is great. It then goes on:

Additionally, it recommends relocating any deleted member function from the private to the public section.

This is great for special member functions. It's not so great for non-special member functions. I think it would be better if that read (and the behavior was):

Additionally, it recommends relocating any deleted special member function from the private to the public section.

The primary use of = delete is for special member functions. We can use it to disable copy and move support on a class. The copyability and moveability of a class is inherently part of the public interface of a class. It makes sense to me for copy and move constructors and assignment operators to be deleted in the public section of a class. The check gets this right.

But = delete has additional uses. It is a very useful way to "override" the C++ overload selection, implicit conversion, and binding rules. On the latter point, it helps us guard against lifetime problems when binding to const-reference function parameters. The clang-tidy check return-const-ref-from-parameter mentions the use of = delete for this.

In these use cases, in my opinion, it makes code far more readable if overloads are grouped together, regardless of whether they are deleted or not. The modernize-use-equals-delete check recommends moving deleted overloads of private functions to the public section of a class, while the undeleted overloads remain private. This feels like overreach. I think the check does this because it is motivated to modernize the pattern of suppressing special member functions, and moving these to the public section is the right thing to do for special member functions, not because it's the right thing in other use cases.

class Foo
{
 public:
  // Skipped

 private:
  const Bar& validate_bar_(const Bar&);
  // Prevent calling the function, above, with an expiring value.
  const Bar& validate_bar_(Bar&&) = delete; // This should remain private, alongside other overloads
};

Related issue: #54276

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions