[PATCH] D20053: [clang-tidy] Add misc-unnecessary-mutable check.

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.

One quick thought: we should probably have a test that includes a macro to make sure nothing explodes. e.g.,

  #define FIELD(T, N) mutable T N
  class C {
    FIELD(int, RemoveMutable);
    FIELD(int, KeepMutable)
  
  public:
    void haha() const {
      KeepMutable = 12;
    }
  };

Not that I expect to run into this case often, but it would be good to ensure the check doesn't crash or attempt to modify the macro.


================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:194
@@ +193,3 @@
+void UnnecessaryMutableCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MD = Result.Nodes.getNodeAs<FieldDecl>("field");
+  const auto *ClassMatch = dyn_cast<CXXRecordDecl>(MD->getParent());
----------------
Why MD; that's usually a method decl.

================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:200
@@ +199,3 @@
+  if (!MD->getDeclName() || ClassMatch->isDependentContext() ||
+      !MD->isMutable())
+    return;
----------------
I think this would be more useful to hoist into the matcher itself (which may mean adding a new AST matcher first, if there isn't already one).


http://reviews.llvm.org/D20053