aaron.ballman added inline comments.
================ Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:58 + << FixItHint::CreateInsertion(StartLoc, "public: ") + << FixItHint::CreateInsertion(AfterLoc, " private:"); +} ---------------- 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. https://reviews.llvm.org/D26138 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
