aaron.ballman added inline comments.
================ Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:29 + cxxMethodDecl( + anyOf(isCopyAssignmentOperator(), isMoveAssignmentOperator())), + cxxDestructorDecl())); ---------------- malcolm.parsons wrote: > 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. I'm okay with that. ================ Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:52 + diag(SpecialFunction->getLocation(), + "use '= delete' to prevent a default special member function") + << FixItHint::CreateInsertion(EndLoc, " = delete"); ---------------- malcolm.parsons wrote: > 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". Given that this is in the modernize module, I think that's reasonable wording. https://reviews.llvm.org/D26138 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
