etienneb added a subscriber: etienneb.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:38
@@ +37,3 @@
+
+ void RunSearch(Decl *Declaration) {
+ auto *Body = Declaration->getBody();
----------------
nit: Decl *Declaration -> const Decl *Declaration
and other visitor functions.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:40
@@ +39,3 @@
+ auto *Body = Declaration->getBody();
+ ParMap = new ParentMap(Body);
+ TraverseStmt(Body);
----------------
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.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:46
@@ +45,3 @@
+ bool VisitExpr(Expr *GenericExpr) {
+ MemberExpr *Expression = dyn_cast<MemberExpr>(GenericExpr);
+ if (Expression == nullptr)
----------------
Could we shortcut the recursion if "FoundNonConstUse" is true.
There is already a case found.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:50
@@ +49,3 @@
+
+ if (Expression->getMemberDecl() != SoughtField)
+ return true;
----------------
this "if" and the previous one should be merged.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:66
@@ +65,3 @@
+ // It's something weird. Just to be sure, assume we're const.
+ IsConstObj = true;
+ } else {
----------------
As soon as something is "IsConstObj", they must be an early exit.
No need to continue iterations.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:80
@@ +79,3 @@
+ // whose parent is ImplicitCastExpr to rvalue or something constant.
+ bool HasRvalueCast = false;
+ bool HasConstCast = false;
----------------
nit: HasRvalueCast -> HasRValueCast
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:85
@@ +84,3 @@
+ dyn_cast<ImplicitCastExpr>(ParMap->getParent(Expression));
+ if (Cast != nullptr) {
+ HasRvalueCast = Cast->getCastKind() == CastKind::CK_LValueToRValue;
----------------
if (Cast != nullptr) {
replace by
if (const auto* Cast =
dyn_cast<ImplicitCastExpr>(ParMap->getParent(Expression)) {
and remove previous line.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:99
@@ +98,3 @@
+
+ bool IsNonConstUseFound() const { return FoundNonConstUse; }
+
----------------
nit: IsNonConstUseFound -> isNonConstUseFound
coding style wants lower case as first letter.
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:121
@@ +120,3 @@
+ FunctionDecl *Declaration = dyn_cast<FunctionDecl>(GenericDecl);
+
+ if (Declaration == nullptr)
----------------
nit: remove this empty line
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:167
@@ +166,3 @@
+
+ if (DeclToken.getKind() == tok::TokenKind::semi) {
+ break;
----------------
nit: remove { }
================
Comment at: clang-tidy/misc/UnnecessaryMutableCheck.cpp:171
@@ +170,3 @@
+
+ if (DeclToken.getKind() == tok::TokenKind::comma) {
+ return false;
----------------
nit: remove { }
http://reviews.llvm.org/D20053
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
