xazax.hun added a comment.
Thanks for working on this, these additions look very useful to me.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:124
+ case Stmt::CXXFunctionalCastExprClass:
+ return cast<CXXFunctionalCastExpr>(Left)->getTypeAsWritten() ==
----------------
You could merge this case with the above case if you cast Left and Right to
ExplicitCastExpr. So both label could end up executing the same code.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:330
+
+AST_MATCHER_P(Expr, expandedByMacro, std::set<std::string>, Names) {
+ const SourceManager &SM = Finder->getASTContext().getSourceManager();
----------------
`llvm::StringSet` might be a more efficient choice.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:336
+ std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
+ if (Names.find(MacroName) != Names.end())
+ return true;
----------------
You could use `count` here.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:357
+ ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
+ return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
+}
----------------
I think you could just return the pointer and return a null pointer in case it
is not an integerConstantExpr. This way no compatibility overload needed.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:510
+// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5
+static void retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
+ BinaryOperatorKind &MainOpcode,
----------------
I would prefer to return a pair of pointer to be returned to output parameters.
================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:579
-AST_MATCHER_P(Expr, expandedByMacro, std::set<std::string>, Names) {
- const SourceManager &SM = Finder->getASTContext().getSourceManager();
- const LangOptions &LO = Finder->getASTContext().getLangOpts();
- SourceLocation Loc = Node.getExprLoc();
- while (Loc.isMacroID()) {
- std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
- if (Names.find(MacroName) != Names.end())
+static bool isOperatorSetMeaningful(BinaryOperatorKind &Opcode,
+ BinaryOperatorKind &LhsOpcode,
----------------
I think a comment might be good here what do you mean by meaningful.
================
Comment at: test/clang-tidy/misc-redundant-expression.cpp:387
// Should not match.
- if (X == 10 && Y == 10) return 1;
- if (X != 10 && X != 12) return 1;
----------------
Why did you remove these test cases?
================
Comment at: test/clang-tidy/misc-redundant-expression.cpp:404
if (X <= 10 && X >= 10) return 1;
- if (!X && !Y) return 1;
- if (!X && Y) return 1;
----------------
Same comment as above.
Repository:
rL LLVM
https://reviews.llvm.org/D38688
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
