[PATCH] Verify assign operator signatures.

Alexander Kornienko alexfh at google.com
Fri Jan 9 05:45:39 PST 2015


Sorry for the long delay.

This looks mostly fine, but I wonder if it makes sense to check for other possible mistakes, e.g.:

  T& operator=(T&);
  const T& operator=(const T&) const;
  virtual T& operator=(const T&);

Also, it seems like this issue is not specific to the Google C++ Style, so it could be put into misc/ or it may even be reasonable to have a compiler warning for this.


================
Comment at: clang-tidy/google/AssignOperatorSignatureCheck.cpp:23
@@ +22,3 @@
+  Finder->addMatcher(
+      methodDecl(unless(isDeleted()), unless(isPrivate()), hasName("operator="),
+                 ofClass(recordDecl().bind("class")),
----------------
I guess you missed "unless(isImplicit())" (and a test for it). And maybe also unless(isInTemplateInstantiation()) (especially, if you would want to provide a fixit for the "const T& operator=(const T&)" case).

================
Comment at: clang-tidy/google/AssignOperatorSignatureCheck.h:25
@@ +24,3 @@
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
----------------
Unfortunately, this needs to be done the old way. LLVM supports MSVS 2013 which doesn't support this form of constructor delegation.

http://reviews.llvm.org/D6667

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list