| Issue |
135249
|
| Summary |
modernize-use-equals-delete should recommend relocating only special members to public
|
| Labels |
new issue
|
| Assignees |
|
| Reporter |
duncanthomson
|
From [the doc](https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-delete.html) 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`](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/return-const-ref-from-parameter.html) 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.
```cpp
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: https://github.com/llvm/llvm-project/issues/54276
![]()
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
