[clang-tools-extra] r256492 - [clang-tidy] Preserve comments and preprocessor directives when simplifying boolean expressions
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 28 05:21:23 PST 2015
Author: alexfh
Date: Mon Dec 28 07:21:22 2015
New Revision: 256492
URL: http://llvm.org/viewvc/llvm-project?rev=256492&view=rev
Log:
[clang-tidy] Preserve comments and preprocessor directives when simplifying boolean expressions
This changeset still emits the diagnostic that the expression could be simplified, but it doesn't generate any fix-its that would lose comments or preprocessor directives within the text that would be replaced.
Fixes PR25842
Reviewers: alexfh
Subscribers: xazax.hun, cfe-commits
Patch by Richard Thomson! (+a naming style fix)
Differential Revision: http://reviews.llvm.org/D15737
Modified:
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=256492&r1=256491&r2=256492&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp Mon Dec 28 07:21:22 2015
@@ -107,8 +107,12 @@ StringRef negatedOperator(const BinaryOp
}
std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {
- {OO_EqualEqual, "=="}, {OO_ExclaimEqual, "!="}, {OO_Less, "<"},
- {OO_GreaterEqual, ">="}, {OO_Greater, ">"}, {OO_LessEqual, "<="}};
+ {OO_EqualEqual, "=="},
+ {OO_ExclaimEqual, "!="},
+ {OO_Less, "<"},
+ {OO_GreaterEqual, ">="},
+ {OO_Greater, ">"},
+ {OO_LessEqual, "<="}};
StringRef getOperatorName(OverloadedOperatorKind OpKind) {
for (auto Name : OperatorNames) {
@@ -201,8 +205,7 @@ std::string replacementExpression(const
}
if (!NegatedOperator.empty() && LHS && RHS) {
return (asBool((getText(Result, *LHS) + " " + NegatedOperator + " " +
- getText(Result, *RHS))
- .str(),
+ getText(Result, *RHS)).str(),
NeedsStaticCast));
}
@@ -324,11 +327,10 @@ void SimplifyBooleanExprCheck::matchExpr
void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
bool Value,
StringRef BooleanId) {
- Finder->addMatcher(
- ifStmt(isExpansionInMainFile(),
- hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId)))
- .bind(IfStmtId),
- this);
+ Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+ hasCondition(cxxBoolLiteral(equals(Value))
+ .bind(BooleanId))).bind(IfStmtId),
+ this);
}
void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
@@ -347,15 +349,13 @@ void SimplifyBooleanExprCheck::matchIfRe
if (ChainedConditionalReturn) {
Finder->addMatcher(ifStmt(isExpansionInMainFile(),
hasThen(returnsBool(Value, ThenLiteralId)),
- hasElse(returnsBool(!Value)))
- .bind(Id),
+ hasElse(returnsBool(!Value))).bind(Id),
this);
} else {
Finder->addMatcher(ifStmt(isExpansionInMainFile(),
unless(hasParent(ifStmt())),
hasThen(returnsBool(Value, ThenLiteralId)),
- hasElse(returnsBool(!Value)))
- .bind(Id),
+ hasElse(returnsBool(!Value))).bind(Id),
this);
}
}
@@ -382,8 +382,7 @@ void SimplifyBooleanExprCheck::matchIfAs
} else {
Finder->addMatcher(ifStmt(isExpansionInMainFile(),
unless(hasParent(ifStmt())), hasThen(Then),
- hasElse(Else))
- .bind(Id),
+ hasElse(Else)).bind(Id),
this);
}
}
@@ -396,8 +395,7 @@ void SimplifyBooleanExprCheck::matchComp
unless(hasElse(stmt())))),
hasAnySubstatement(
returnStmt(has(cxxBoolLiteral(equals(!Value))))
- .bind(CompoundReturnId))))
- .bind(Id),
+ .bind(CompoundReturnId)))).bind(Id),
this);
}
@@ -490,6 +488,39 @@ void SimplifyBooleanExprCheck::check(con
}
}
+bool containsDiscardedTokens(
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ CharSourceRange CharRange) {
+ StringRef ReplacementText =
+ Lexer::getSourceText(CharRange, *Result.SourceManager,
+ Result.Context->getLangOpts()).str();
+ Lexer Lex(CharRange.getBegin(), Result.Context->getLangOpts(),
+ ReplacementText.data(), ReplacementText.data(),
+ ReplacementText.data() + ReplacementText.size());
+ Lex.SetCommentRetentionState(true);
+ Token Tok;
+
+ while (!Lex.LexFromRawLexer(Tok)) {
+ if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))
+ return true;
+ }
+
+ return false;
+}
+
+void SimplifyBooleanExprCheck::issueDiag(
+ const ast_matchers::MatchFinder::MatchResult &Result, SourceLocation Loc,
+ StringRef Description, SourceRange ReplacementRange,
+ StringRef Replacement) {
+ CharSourceRange CharRange = Lexer::makeFileCharRange(
+ CharSourceRange::getTokenRange(ReplacementRange), *Result.SourceManager,
+ Result.Context->getLangOpts());
+
+ DiagnosticBuilder Diag = diag(Loc, Description);
+ if (!containsDiscardedTokens(Result, CharRange))
+ Diag << FixItHint::CreateReplacement(CharRange, Replacement);
+}
+
void SimplifyBooleanExprCheck::replaceWithExpression(
const ast_matchers::MatchFinder::MatchResult &Result,
const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) {
@@ -497,19 +528,18 @@ void SimplifyBooleanExprCheck::replaceWi
const auto *RHS = Result.Nodes.getNodeAs<Expr>(RHSId);
std::string Replacement =
replacementExpression(Result, Negated, UseLHS ? LHS : RHS);
- SourceLocation Start = LHS->getLocStart();
- SourceLocation End = RHS->getLocEnd();
- diag(BoolLiteral->getLocStart(), SimplifyOperatorDiagnostic)
- << FixItHint::CreateReplacement(SourceRange(Start, End), Replacement);
+ SourceRange Range(LHS->getLocStart(), RHS->getLocEnd());
+ issueDiag(Result, BoolLiteral->getLocStart(), SimplifyOperatorDiagnostic,
+ Range, Replacement);
}
void SimplifyBooleanExprCheck::replaceWithThenStatement(
const MatchFinder::MatchResult &Result,
const CXXBoolLiteralExpr *TrueConditionRemoved) {
const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
- diag(TrueConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
- << FixItHint::CreateReplacement(IfStatement->getSourceRange(),
- getText(Result, *IfStatement->getThen()));
+ issueDiag(Result, TrueConditionRemoved->getLocStart(),
+ SimplifyConditionDiagnostic, IfStatement->getSourceRange(),
+ getText(Result, *IfStatement->getThen()));
}
void SimplifyBooleanExprCheck::replaceWithElseStatement(
@@ -517,10 +547,9 @@ void SimplifyBooleanExprCheck::replaceWi
const CXXBoolLiteralExpr *FalseConditionRemoved) {
const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
const Stmt *ElseStatement = IfStatement->getElse();
- diag(FalseConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
- << FixItHint::CreateReplacement(
- IfStatement->getSourceRange(),
- ElseStatement ? getText(Result, *ElseStatement) : "");
+ issueDiag(Result, FalseConditionRemoved->getLocStart(),
+ SimplifyConditionDiagnostic, IfStatement->getSourceRange(),
+ ElseStatement ? getText(Result, *ElseStatement) : "");
}
void SimplifyBooleanExprCheck::replaceWithCondition(
@@ -528,9 +557,9 @@ void SimplifyBooleanExprCheck::replaceWi
bool Negated) {
std::string Replacement =
replacementExpression(Result, Negated, Ternary->getCond());
- diag(Ternary->getTrueExpr()->getLocStart(),
- "redundant boolean literal in ternary expression result")
- << FixItHint::CreateReplacement(Ternary->getSourceRange(), Replacement);
+ issueDiag(Result, Ternary->getTrueExpr()->getLocStart(),
+ "redundant boolean literal in ternary expression result",
+ Ternary->getSourceRange(), Replacement);
}
void SimplifyBooleanExprCheck::replaceWithReturnCondition(
@@ -540,8 +569,8 @@ void SimplifyBooleanExprCheck::replaceWi
std::string Replacement = ("return " + Condition + Terminator).str();
SourceLocation Start =
Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(ThenLiteralId)->getLocStart();
- diag(Start, SimplifyConditionalReturnDiagnostic)
- << FixItHint::CreateReplacement(If->getSourceRange(), Replacement);
+ issueDiag(Result, Start, SimplifyConditionalReturnDiagnostic,
+ If->getSourceRange(), Replacement);
}
void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
@@ -570,10 +599,9 @@ void SimplifyBooleanExprCheck::replaceCo
const Expr *Condition = If->getCond();
std::string Replacement =
"return " + replacementExpression(Result, Negated, Condition);
- diag(Lit->getLocStart(), SimplifyConditionalReturnDiagnostic)
- << FixItHint::CreateReplacement(
- SourceRange(If->getLocStart(), Ret->getLocEnd()),
- Replacement);
+ issueDiag(
+ Result, Lit->getLocStart(), SimplifyConditionalReturnDiagnostic,
+ SourceRange(If->getLocStart(), Ret->getLocEnd()), Replacement);
return;
}
@@ -598,8 +626,9 @@ void SimplifyBooleanExprCheck::replaceWi
(VariableName + " = " + Condition + Terminator).str();
SourceLocation Location =
Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(IfAssignLocId)->getLocStart();
- this->diag(Location, "redundant boolean literal in conditional assignment")
- << FixItHint::CreateReplacement(Range, Replacement);
+ issueDiag(Result, Location,
+ "redundant boolean literal in conditional assignment", Range,
+ Replacement);
}
} // namespace readability
Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h?rev=256492&r1=256491&r2=256492&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h Mon Dec 28 07:21:22 2015
@@ -154,6 +154,10 @@ private:
const ast_matchers::MatchFinder::MatchResult &Result,
const CompoundStmt *Compound, bool Negated = false);
+ void issueDiag(const ast_matchers::MatchFinder::MatchResult &Result,
+ SourceLocation Loc, StringRef Description,
+ SourceRange ReplacementRange, StringRef Replacement);
+
const bool ChainedConditionalReturn;
const bool ChainedConditionalAssignment;
};
Modified: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp?rev=256492&r1=256491&r2=256492&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp Mon Dec 28 07:21:22 2015
@@ -871,3 +871,27 @@ bool negated_null_pointer_condition(int
}
// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
// CHECK-FIXES: return p4 != nullptr;{{$}}
+
+bool comments_in_the_middle(bool b) {
+ if (b) {
+ return true;
+ } else {
+ // something wicked this way comes
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}} if (b) {
+// CHECK-FIXES: // something wicked this way comes{{$}}
+
+bool preprocessor_in_the_middle(bool b) {
+ if (b) {
+ return true;
+ } else {
+#define SOMETHING_WICKED false
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}} if (b) {
+// CHECK-FIXES: {{^}}#define SOMETHING_WICKED false
More information about the cfe-commits
mailing list