[clang-tidy] Add misc-unnecessary-mutable check.

[mnbvmar]

Summary

This implements unnecessary-mutable check. It's still bug-prone and might produce false positives, so all suggestions are welcome.

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

[alexfh]

docs/clang-tidy/checks/misc-unnecessary-mutable.rst

10

Please verify the documentation can be built without errors. On Ubuntu this boils down to:

Install sphinx:

1. $ sudo apt-get install sphinx-common
2. Enable LLVM_BUILD_DOCS and maybe some other options in cmake.
3. $ ninja docs-clang-tools-html (or something similar, if you use make).

[aaron.ballman]

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.

clang-tidy/misc/UnnecessaryMutableCheck.cpp

195

Why MD; that's usually a method decl.

201

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).

[Prazek]

clang-tidy/misc/UnnecessaryMutableCheck.cpp

153

I guess you can use hasSingleDecl in matcher to solve this

[etienneb]

clang-tidy/misc/UnnecessaryMutableCheck.cpp

39

nit: Decl *Declaration -> const Decl *Declaration

and other visitor functions.

41

Why not using stack memory instead of allocating memory?

ParentMap LocalMap;
ParMap = &LocalMap;
TraverseStmt(Body);
ParMap = nullptr; /// <-- I also prefer seeing this variable being after recursion.

47

Could we shortcut the recursion if "FoundNonConstUse" is true.
There is already a case found.

51

this "if" and the previous one should be merged.

67

As soon as something is "IsConstObj", they must be an early exit.
No need to continue iterations.

81

nit: HasRvalueCast -> HasRValueCast

86

if (Cast != nullptr) {

replace by

if (const auto* Cast = dyn_cast<ImplicitCastExpr>(ParMap->getParent(Expression)) {

and remove previous line.

100

nit: IsNonConstUseFound -> isNonConstUseFound
coding style wants lower case as first letter.

122

nit: remove this empty line

168

nit: remove { }

172

nit: remove { }

[aaron.ballman]

test/clang-tidy/misc-unnecessary-mutable.cpp

238

It would be good to put further information in about why this fails.

[mnbvmar]

Fixes done.
Added macro test.
Docs should be working now.
Updated docs.

[mnbvmar]

clang-tidy/misc/UnnecessaryMutableCheck.cpp

47

Unless I miss something, the moment we set FoundNonConstUse to true, we stop recurring both in FieldUseVisitor and ClassMethodVisitor.

[etienneb]

clang-tidy/misc/UnnecessaryMutableCheck.cpp

23

add an anonymous namespace around this declaration.

26

why getDeclName ?

42

Please add a comment to describe the purpose of this class.
The code is the doc, and it will help readers.

56

MemberExpr *Expression -> const auto*

60

thing -> object

62

I think the child-expressions of MemberExpr can only be "member->getBase()" ?
In this case, no loop is needed.

child_range children() { return child_range(&Base, &Base+1); }

Expr *getBase() const { return cast<Expr>(Base); }

134

Watch out, Declaration->hasBody() is tricky with code when compile on windows (clang-cl).
hasBody() may return true, but the getBody() may be NULL.

This is why these tests exist:

cppcoreguidelines-pro-type-member-init-delayed.cpp
modernize-redundant-void-arg-delayed.cpp
modernize-use-default-delayed.cpp
performance-unnecessary-value-param-delayed.cpp

So this code may crash with delayed-template-parsing.

213

no need for ->getDeclName()
There is an overloaded operator for namedDecl.

217

You can change CheckRemoval to return the SourceRange.
If it's failing, you can call 'fixithint <<' and it won't be an issue.

This way, you do not need to declare Diag, and you can directly add the sourceRange to the expression to line 213.

diag(FD->getLocation(), "'mutable' modifier is unnecessary for field %0")
 << FD
 << getSourceRangeOfMutableToken(FD);

WDYT?

clang-tidy/misc/CMakeLists.txt

   ThrowByValueCatchByReferenceCheck.cpp
   UndelegatedConstructor.cpp
   UniqueptrResetReleaseCheck.cpp
+  UnnecessaryMutableCheck.cpp
   UnusedAliasDeclsCheck.cpp
   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp

clang-tidy/misc/MiscTidyModule.cpp

 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
+#include "UnnecessaryMutableCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedRAIICheck.h"

         "misc-undelegated-constructor");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
         "misc-uniqueptr-reset-release");
+    CheckFactories.registerCheck<UnnecessaryMutableCheck>(
+        "misc-unnecessary-mutable");
     CheckFactories.registerCheck<UnusedAliasDeclsCheck>(
         "misc-unused-alias-decls");
     CheckFactories.registerCheck<UnusedParametersCheck>(

clang-tidy/misc/UnnecessaryMutableCheck.h

+//===--- UnnecessaryMutableCheck.h - clang-tidy------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNNECESSARY_MUTABLE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNNECESSARY_MUTABLE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Searches for and removes unnecessary 'mutable' keywords (that is, for
+/// fields that cannot be possibly changed in const methods).
+///
+/// Example:
+///
+/// class MyClass {
+///   mutable int field;  // will change into "int field;"
+/// };
+///
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-unnecessary-mutable.html
+class UnnecessaryMutableCheck : public ClangTidyCheck {
+public:
+  UnnecessaryMutableCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNNECESSARY_MUTABLE_H

clang-tidy/misc/UnnecessaryMutableCheck.cpp

+//===--- UnnecessaryMutableCheck.cpp - clang-tidy--------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnnecessaryMutableCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+// Matcher checking if the declaration is non-macro existent mutable which is
+// not dependent on context.
+AST_MATCHER(FieldDecl, isSubstantialMutable) {
+  return Node.getDeclName() && Node.isMutable() &&
+         !Node.getLocation().isMacroID() &&
+         !Node.getParent()->isDependentContext();
+}
+
+void UnnecessaryMutableCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      fieldDecl(anyOf(isPrivate(), allOf(isProtected(),
+                                         hasParent(cxxRecordDecl(isFinal())))),
+                unless(anyOf(isImplicit(), isInstantiated(),
+                             hasParent(cxxRecordDecl(isUnion())))),
+                isSubstantialMutable())
+          .bind("field"),
+      this);
+}
+
+class FieldUseVisitor : public RecursiveASTVisitor<FieldUseVisitor> {
+public:
+  explicit FieldUseVisitor(const FieldDecl *SoughtField)
+      : SoughtField(SoughtField), FoundNonConstUse(false) {}
+
+  void RunSearch(const Decl *Declaration) {
+    auto *Body = Declaration->getBody();
+    ParentMap LocalMap(Body);
+    ParMap = &LocalMap;
+    TraverseStmt(Body);
+    ParMap = nullptr;
+  }
+
+  bool VisitExpr(Expr *GenericExpr) {
+    MemberExpr *Expression = dyn_cast<MemberExpr>(GenericExpr);
+    if (!Expression || Expression->getMemberDecl() != SoughtField)
+      return true;
+
+    // Check if expr is a member of const thing.
+    bool IsConstObj = false;
+    for (const auto *ChildStmt : Expression->children()) {
+      const Expr *ChildExpr = dyn_cast<Expr>(ChildStmt);
+      if (ChildExpr) {
+        IsConstObj |= ChildExpr->getType().isConstQualified();
+
+        // If member expression dereferences, we need to check
+        // whether pointer type is const.
+        if (Expression->isArrow()) {
+          const auto &WrappedType = *ChildExpr->getType();
+          if (!WrappedType.isPointerType()) {
+            // It's something weird. Just to be sure, assume we're const.
+            IsConstObj = true;
+          } else {
+            IsConstObj |= WrappedType.getPointeeType().isConstQualified();
+          }
+        }
+      }
+
+      if (IsConstObj)
+        break;
+    }
+
+    // If it's not, mutableness changes nothing.
+    if (!IsConstObj)
+      return true;
+
+    // By a const operation on a member expression we mean a MemberExpr
+    // whose parent is ImplicitCastExpr to rvalue or something constant.
+    bool HasRValueCast = false;
+    bool HasConstCast = false;
+    if (ParMap->hasParent(Expression)) {
+      if (const auto *Cast =
+              dyn_cast<ImplicitCastExpr>(ParMap->getParent(Expression))) {
+        HasRValueCast = Cast->getCastKind() == CastKind::CK_LValueToRValue;
+        HasConstCast = Cast->getType().isConstQualified();
+      }
+    }
+
+    if (!HasRValueCast && !HasConstCast) {
+      FoundNonConstUse = true;
+      return false;
+    }
+
+    return true;
+  }
+
+  bool isNonConstUseFound() const { return FoundNonConstUse; }
+
+private:
+  const FieldDecl *SoughtField;
+  ParentMap *ParMap;
+  bool FoundNonConstUse;
+};
+
+class ClassMethodVisitor : public RecursiveASTVisitor<ClassMethodVisitor> {
+public:
+  ClassMethodVisitor(ASTContext &Context, const FieldDecl *SoughtField)
+      : SM(Context.getSourceManager()), SoughtField(SoughtField),
+        NecessaryMutable(false) {}
+
+  bool VisitDecl(const Decl *GenericDecl) {
+    // As for now, we can't track friends.
+    if (dyn_cast<FriendDecl>(GenericDecl)) {
+      NecessaryMutable = true;
+      return false;
+    }
+
+    const FunctionDecl *Declaration = dyn_cast<FunctionDecl>(GenericDecl);
+    if (Declaration == nullptr)
+      return true;
+
+    // All decls need their definitions in main file.
+    if (!Declaration->hasBody() ||
+        !SM.isInMainFile(Declaration->getBody()->getLocStart())) {
+      NecessaryMutable = true;
+      return false;
+    }
+
+    FieldUseVisitor FieldVisitor(SoughtField);
+    FieldVisitor.RunSearch(Declaration);
+    if (FieldVisitor.isNonConstUseFound()) {
+      NecessaryMutable = true;
+      return false;
+    }
+
+    return true;
+  }
+
+  bool IsMutableNecessary() const { return NecessaryMutable; }
+
+private:
+  SourceManager &SM;
+  const FieldDecl *SoughtField;
+  bool NecessaryMutable;
+};
+
+// Checks if 'mutable' keyword can be removed; for now; we do it only if
+// it is the only declaration in a declaration chain.
+static bool CheckRemoval(SourceManager &SM, const SourceLocation &LocStart,
+                         const SourceLocation &LocEnd, ASTContext &Context,
+                         SourceRange &ResultRange) {
+
+  const FileID FID = SM.getFileID(LocEnd);
+  const llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, LocEnd);
+  Lexer DeclLexer(SM.getLocForStartOfFile(FID), Context.getLangOpts(),
+                  Buffer->getBufferStart(), SM.getCharacterData(LocStart),
+                  Buffer->getBufferEnd());
+
+  Token DeclToken;
+  bool result = false;
+
+  while (!DeclLexer.LexFromRawLexer(DeclToken)) {
+
+    if (DeclToken.getKind() == tok::TokenKind::semi)
+      break;
+
+    if (DeclToken.getKind() == tok::TokenKind::comma)
+      return false;
+
+    if (DeclToken.isOneOf(tok::TokenKind::identifier,
+                          tok::TokenKind::raw_identifier)) {
+      auto TokenStr = DeclToken.getRawIdentifier().str();
+
+      // "mutable" cannot be used in any other way than to mark mutableness
+      if (TokenStr == "mutable") {
+        ResultRange =
+            SourceRange(DeclToken.getLocation(), DeclToken.getEndLoc());
+        result = true;
+      }
+    }
+  }
+
+  assert(result && "No mutable found, weird");
+
+  return result;
+}
+
+void UnnecessaryMutableCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *FD = Result.Nodes.getNodeAs<FieldDecl>("field");
+  const auto *ClassMatch = dyn_cast<CXXRecordDecl>(FD->getParent());
+  auto &Context = *Result.Context;
+  auto &SM = *Result.SourceManager;
+
+  ClassMethodVisitor Visitor(Context, FD);
+  Visitor.TraverseDecl(const_cast<CXXRecordDecl *>(ClassMatch));
+
+  if (Visitor.IsMutableNecessary())
+    return;
+
+  auto Diag =
+      diag(FD->getLocation(), "'mutable' modifier is unnecessary for field %0")
+      << FD->getDeclName();
+
+  SourceRange RemovalRange;
+
+  if (CheckRemoval(SM, FD->getLocStart(), FD->getLocEnd(), Context,
+                   RemovalRange)) {
+    Diag << FixItHint::CreateRemoval(RemovalRange);
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

docs/ReleaseNotes.rst

 
   Find suspicious usage of runtime string comparison functions.
 
+- New `misc-unnecessary-mutable
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-unnecessary-mutable.html>`_ check
+
+  Finds places where ``mutable`` keyword is potentially unneeded.
+
 - New `misc-unused-using-decls
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-unused-using-decls.html>`_ check
 

docs/clang-tidy/checks/list.rst

    misc-unconventional-assign-operator
    misc-undelegated-constructor
    misc-uniqueptr-reset-release
+   misc-unnecessary-mutable
    misc-unused-alias-decls
    misc-unused-parameters
    misc-unused-raii

docs/clang-tidy/checks/misc-unnecessary-mutable.rst

+.. title:: clang-tidy - misc-unnecessary-mutable
+
+misc-unnecessary-mutable
+========================
+
+The tool tries to find situations where ``mutable`` might be unnecessary
+(i.e. each use of the mutable field on the const object is constant).
+
+.. code-block:: c++
+
+  class SomeClass {
+    public:
+      void foo() {
+        field2 = 42;
+      }
+
+      void bar() const {
+        field3 = 99;
+      }
+
+      // Should stay mutable. Everyone can change it.
+      mutable int field1;
+
+    private:
+      // Might lose 'mutable'. It's never modified in const object.
+      mutable int field2;
+
+      // Should stay mutable; bar() changes it even though 'this' is const.
+      mutable int field3;
+  };
+
+It also can automatically remove ``mutable`` keyword. For now, it is done
+only if non-necessarily mutable field is the only declaration in the statement.
+
+Please note that there is a possibility that false-positives occur. For example,
+one can use ``const_cast``:
+
+.. code-block:: c++
+
+  class AnotherClass {
+    public:
+      void foo() const {
+        const int& y = x;
+        const_cast<int&>(y) = 42;
+      }
+
+    private:
+      mutable int x; // SHOULD stay mutable. For now, it does not.
+  };
+
+The tool should be thus used with care.

test/clang-tidy/misc-unnecessary-mutable.cpp

+// RUN: %check_clang_tidy %s misc-unnecessary-mutable %t
+
+struct NothingMutable {
+  int field1;
+  unsigned field2;
+  const int field3;
+  volatile float field4;
+
+  NothingMutable(int a1, unsigned a2, int a3, float a4) : field1(a1), field2(a2), field3(a3), field4(a4) {}
+
+  void doSomething() {
+    field1 = 1;
+    field2 = 2;
+    field4 = 4;
+  }
+};
+
+struct NoMethods {
+  int field1;
+  mutable unsigned field2; // These cannot be fixed; they're public
+  const int field3;
+  mutable volatile NothingMutable field4;
+};
+
+class NoMethodsClass {
+public:
+  int field1;
+  mutable unsigned field2;
+
+private:
+  const int field3;
+  mutable volatile NothingMutable field4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: 'mutable' modifier is unnecessary for field 'field4' [misc-unnecessary-mutable]
+  // CHECK-FIXES: {{^  }}volatile NothingMutable field4;
+};
+
+struct PrivateInStruct {
+private:
+  mutable volatile unsigned long long blah;
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 'mutable' modifier is unnecessary for field 'blah' {{..}}
+  // CHECK-FIXES: {{^  }}volatile unsigned long long blah;
+};
+
+union PrivateInUnion {
+public:
+  int someField;
+
+private:
+  mutable char otherField;
+};
+
+class UnusedVar {
+private:
+  mutable int x __attribute__((unused));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'mutable' modifier is unnecessary for field 'x' {{..}}
+  // CHECK-FIXES: {{^  }}int x __attribute__((unused));
+};
+
+class NoConstMethodsClass {
+public:
+  int field1;
+  mutable unsigned field2;
+
+  NoConstMethodsClass() : field2(42), field3(9), field4(NothingMutable(1, 2, 3, 4)) {}
+
+  void doSomething() {
+    field2 = 8;
+    field1 = 99;
+    field4.doSomething();
+  }
+
+private:
+  const int field3;
+  mutable NothingMutable field4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'mutable' modifier is unnecessary for field 'field4' {{..}}
+  // CHECK-FIXES: {{^  }}NothingMutable field4;
+};
+
+class ConstMethods {
+private:
+  mutable int field1, field2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'mutable' modifier is unnecessary for field 'field2' {{..}}
+  mutable int incr, decr, set, mul, constArg1, constArg2, constRef, ref1, ref2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'mutable' modifier is unnecessary for field 'constArg1' {{..}}
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'mutable' modifier is unnecessary for field 'constArg2' {{..}}
+  // CHECK-MESSAGES: :[[@LINE-3]]:59: warning: 'mutable' modifier is unnecessary for field 'constRef' {{..}}
+
+  void takeArg(int x) const { x *= 4; }
+  int takeConstRef(const int &x) const { return x + 99; }
+  void takeRef(int &) const {}
+
+  template <typename... Args>
+  void takeArgs(Args... args) const {}
+  template <typename... Args>
+  void takeArgRefs(Args &... args) const {}
+
+public:
+  void doSomething() const {
+    field1 = field2;
+  }
+
+  void doOtherThing() const {
+    incr++;
+    decr--;
+    set = 42;
+    mul *= 3;
+    takeArg(constArg1);
+    takeConstRef(constRef);
+    takeRef(ref1);
+    takeArgs(constArg1, constArg2);
+    takeArgRefs(ref1, ref2);
+  }
+};
+
+class NonFinalClass {
+public:
+  mutable int fPublic;
+
+protected:
+  mutable int fProtected;
+
+private:
+  mutable int fPrivate;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'mutable' modifier is unnecessary for field 'fPrivate' {{..}}
+  // CHECK-FIXES: {{^  }}int fPrivate;
+};
+
+class FinalClass final {
+public:
+  mutable int fPublic;
+
+protected:
+  mutable int fProtected;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'mutable' modifier is unnecessary for field 'fProtected' {{..}}
+  // CHECK-FIXES: {{^  }}int fProtected;
+
+private:
+  mutable int fPrivate;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'mutable' modifier is unnecessary for field 'fPrivate' {{..}}
+  // CHECK-FIXES: {{^  }}int fPrivate;
+};
+
+class NotAllFuncsKnown {
+  void doSomething();
+  void doSomethingConst() const {}
+
+private:
+  mutable int field;
+  // Can't be fixed. We don't know if doSomething() doesn't declare a *const* NotAllFuncsKnown instance
+  // and then modify 'field' field.
+};
+
+class NotAllConstFuncsKnown {
+  void doSomething() {}
+  void doSomethingConst() const;
+  void doOtherConst() const {}
+
+private:
+  mutable int field;
+};
+
+class ConstFuncOutside {
+  void doSomething();
+  void doSomethingConst() const;
+  void doOtherConst() const {}
+
+private:
+  mutable int field;
+};
+
+void ConstFuncOutside::doSomethingConst() const {}
+
+// We can't really see if mutable is necessary or not.
+template <typename T>
+class TemplatedClass {
+public:
+  void doSomething() {
+    a = b = c;
+  }
+
+  void doSomethingConst() const {
+    a = b = c;
+  }
+
+private:
+  mutable int a, b, c;
+};
+
+TemplatedClass<int> TemplatedClassInt;
+
+class ClassWithTemplates {
+public:
+  void doSomethingConst() const {
+    a = b;
+  }
+
+  // Here, we can see that.
+  template <typename T>
+  void doOtherConst() const {
+    b = c;
+  }
+
+private:
+  mutable int a, b, c, d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'mutable' modifier is unnecessary for field 'c' {{..}}
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: 'mutable' modifier is unnecessary for field 'd' {{..}}
+};
+
+class ImplicitCast {
+public:
+  void doSomethingConst() const {
+    a = b;
+  }
+
+private:
+  mutable int a;
+  mutable double b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'mutable' modifier is unnecessary for field 'b' {{..}}
+  // CHECK_FIXES: {{^  }}double b;
+};
+
+struct MutableNotFirst {
+private:
+  long mutable long abc = 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'mutable' modifier is unnecessary for field 'abc' {{..}}
+  // CHECK_FIXES: {{^  }}long long abc;
+  long long mutable bca;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'mutable' modifier is unnecessary for field 'bca' {{..}}
+  // CHECK_FIXES: {{^  }}long long bca;
+  int mutable ca;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'mutable' modifier is unnecessary for field 'ca' {{..}}
+  // CHECK_FIXES: {{^  }}int ca;
+};
+
+// Fails for now. (See: documentation.)
+/*
+void change(const int &a) {
+    const_cast<int&>(a) = 42;
+}
+
+struct Change {
+    void changeMember() const {
+        change(m);
+        const int& x = otherM;
+        const_cast<int&>(x) = 33;
+
+        const_cast<int&>(anotherM) = -4;
+    }
+private:
+    mutable int m;        
+    mutable int otherM;
+    mutable int anotherM;
+};
+*/
+
+class StrangeClass {
+public:
+  void foo() {}
+};
+
+void EvilFunction(int &a) {}
+void EvilFunction(const int &a) {}
+
+class JustClass {
+public:
+  JustClass() {}
+
+  void foo() {
+    MutableClass.foo();
+  }
+
+  void foo() const;
+  void evilCaller() const;
+
+private:
+  mutable StrangeClass MutableClass; // Must stay mutable (because of foo func below)
+  mutable int MutableInt;            // Must stay mutable
+};
+
+void JustClass::foo() const {
+  MutableClass.foo();
+}
+
+void JustClass::evilCaller() const {
+  EvilFunction(MutableInt);
+}
+
+class AnotherStrangeClass {
+public:
+  // Example of non-const method which requires that MutableInt should stay mutable.
+  void strangeFoo() {
+    const AnotherStrangeClass *ConstThis = this;
+    EvilFunction(ConstThis->MutableInt);
+  }
+
+private:
+  mutable int MutableInt; // must stay mutable
+};
+
+class ClassWithStrangeConstructor {
+public:
+  ClassWithStrangeConstructor() {
+    const ClassWithStrangeConstructor *ConstThis = this;
+    EvilFunction(ConstThis->MutableInt);
+  }
+
+private:
+  mutable int MutableInt; // must stay mutable
+};
+
+class ClassWithStrangeDestructor {
+public:
+  ~ClassWithStrangeDestructor() {
+    const ClassWithStrangeDestructor *ConstThis = this;
+    EvilFunction(ConstThis->MutableInt);
+  }
+
+private:
+  mutable int MutableInt; // must stay mutable
+};
+
+// Don't touch friends or they'll hurt you.
+class ClassWithFriends {
+public:
+  friend void someFriend(ClassWithFriends *);
+
+private:
+  mutable int MutableInt; // must stay mutable
+};
+
+void someFriend(ClassWithFriends *Class) {
+  const ClassWithFriends *ConstClass = Class;
+  EvilFunction(ConstClass->MutableInt);
+}
+
+class ClassWithClassFriends {
+public:
+  friend class ClassFriend;
+
+private:
+  mutable int MutableInt; // must stay mutable
+};
+
+class ClassFriend {
+public:
+  static void foo(const ClassWithClassFriends *Class) {
+    EvilFunction(Class->MutableInt);
+  }
+};
+
+
+// Definitions containing macros. Nothing inside macro should
+// be noticed nor modified. However, if 'mutable' keyword is outside
+// of our macro, we leave the freedom to the check. It is not expected
+// to notice and fix, but if it does (and it does once below), it is
+// even better for us.
+// This test was added mainly to figure out if nothing explodes inside the
+// check.
+
+#define MUTABLE_FIELD(TYPE, NAME) mutable TYPE NAME
+#define SOME_FIELD(TYPE, NAME) TYPE NAME
+#define SOME_FIELD_SEMI(TYPE, NAME) TYPE NAME;
+#define SOME_FIELD_TYPE(TYPE) TYPE
+class MacroFields {
+  MUTABLE_FIELD(int, removedMutable);
+  MUTABLE_FIELD(int, keptMutable);
+  mutable SOME_FIELD(int, removedMutable2);
+  mutable SOME_FIELD_SEMI(int, removedMutable3)
+  mutable SOME_FIELD_TYPE(int) removedMutable4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'mutable' modifier is unnecessary for field 'removedMutable4' {{..}}
+  // CHECK-FIXES: {{^  }}SOME_FIELD_TYPE(int) removedMutable4;
+
+public:
+  void modify() const {
+    keptMutable = 44;
+  }
+};