[PATCH] D26138: [clang-tidy] Add modernize-use-equals-delete check

[Aaron Ballman via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!



================
Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:58
+      << FixItHint::CreateInsertion(StartLoc, "public: ")
+      << FixItHint::CreateInsertion(AfterLoc, " private:");
+}
----------------
malcolm.parsons wrote:
> aaron.ballman wrote:
> > malcolm.parsons wrote:
> > > aaron.ballman wrote:
> > > > I am on the fence about this fixit. On the one hand, the fix is a technical improvement because it means that implementations will consistently find the declaration and bark about it being explicitly deleted. On the other hand, the fixit suggests code that should never pass a reasonable code review.
> > > > 
> > > > I'm wondering if it would make more sense to leave the access specifiers alone and just put a FIXME in the code to point the situation out. I am guessing that at some point we will have a refactoring tool that can help without resorting to making declarations like `public: C() = delete; private:`.
> > > > 
> > > > What do you think?
> > > I'm wondering whether no fixit would be better than a not-good-enough fixit.
> > Generally, no. Incremental improvements are almost always fine. However, the user is asking to have their code modernized, and the fixit results in code that looks more foreign than modern (at least, to me).
> > 
> > I won't block the patch moving forward regardless of whether the fixit is in or out, but I am curious if @alexfh has an opinion, or if you have a strong preference one way or the other.
> I'll change the check to warn about deleted methods that aren't public; the user can fix those manually until a better fixit is possible.
I think that's a good solution for now, thanks!


https://reviews.llvm.org/D26138