malcolm.parsons added inline comments.
================ Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:29 + cxxMethodDecl( + anyOf(isCopyAssignmentOperator(), isMoveAssignmentOperator())), + cxxDestructorDecl())); ---------------- aaron.ballman wrote: > malcolm.parsons wrote: > > aaron.ballman wrote: > > > How about a conversion operator, like `operator bool()`? You'll sometimes > > > see that one declared privately for similar reasons. > > I haven't seen that. Do you have an example? > anecdote != data, and all that, but: > http://stackoverflow.com/questions/5753460/a-way-to-disable-conversion-operators > > I do agree though, this is not as common as noncopyable classes. I think I'll leave conversion operators to future modernize-use-explicit-conversion-operators or cppcoreguidelines-avoid-conversion-operators checks. ================ Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:52 + diag(SpecialFunction->getLocation(), + "use '= delete' to prevent a default special member function") + << FixItHint::CreateInsertion(EndLoc, " = delete"); ---------------- aaron.ballman wrote: > malcolm.parsons wrote: > > aaron.ballman wrote: > > > This diagnostic isn't very clear to me -- what does it mean to "prevent" > > > a default special member function? > > > > > > The fixit for this is also somewhat unsatisfying as this takes a private, > > > not-defined function and turns it into a private, deleted function. > > > That's a very small win, because it only impacts code which could access > > > the special member function in the first place (some compilers give a > > > diagnostic about the special member function being inaccessible even if > > > it's explicitly marked as deleted; clang is not one such compiler). Do we > > > have a way to rewrite the access specifier for the special member > > > function as well (kind of like how we have a way to handle includes we're > > > adding)? I am guessing not yet, but if we do, that would be fantastic to > > > use here. > > > > > > Note, I don't think this should hold up your patch or the fixit. A small > > > win is still a win. :-) > > Do you have a better wording for the diagnostic? > > > > I don't see any utility functions to make a method public. > Perhaps: "special member function with private access specifier and no > definition is still accessible; use '= delete' to explicitly disallow all > access"? > > Or a less-wordy variant. How about "use '= delete' to prohibit calling of a special member function". https://reviews.llvm.org/D26138 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
