Author: alexfh Date: Thu May 4 10:34:23 2017 New Revision: 302160 URL: http://llvm.org/viewvc/llvm-project?rev=302160&view=rev Log: [clang-tidy] Code cleanup, (almost) NFC (*).
(*) Printed types of member pointers don't use elaborated type specifiers (`int struct S::*` -> `int S::*`). Modified: clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp?rev=302160&r1=302159&r2=302160&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp Thu May 4 10:34:23 2017 @@ -11,6 +11,7 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" #include <queue> using namespace clang::ast_matchers; @@ -32,7 +33,7 @@ bool isNULLMacroExpansion(const Stmt *St const LangOptions &LO = Context.getLangOpts(); SourceLocation Loc = Statement->getLocStart(); return SM.isMacroBodyExpansion(Loc) && - clang::Lexer::getImmediateMacroName(Loc, SM, LO) == "NULL"; + Lexer::getImmediateMacroName(Loc, SM, LO) == "NULL"; } AST_MATCHER(Stmt, isNULLMacroExpansion) { @@ -57,17 +58,15 @@ StatementMatcher createImplicitCastFromB hasSourceExpression(expr(hasType(qualType(booleanType()))))); } -StringRef -getZeroLiteralToCompareWithForGivenType(CastKind CastExpressionKind, - QualType CastSubExpressionType, - ASTContext &Context) { - switch (CastExpressionKind) { +StringRef getZeroLiteralToCompareWithForType(CastKind CastExprKind, + QualType Type, + ASTContext &Context) { + switch (CastExprKind) { case CK_IntegralToBoolean: - return CastSubExpressionType->isUnsignedIntegerType() ? "0u" : "0"; + return Type->isUnsignedIntegerType() ? "0u" : "0"; case CK_FloatingToBoolean: - return Context.hasSameType(CastSubExpressionType, Context.FloatTy) ? "0.0f" - : "0.0"; + return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0"; case CK_PointerToBoolean: case CK_MemberPointerToBoolean: // Fall-through on purpose. @@ -79,10 +78,8 @@ getZeroLiteralToCompareWithForGivenType( } bool isUnaryLogicalNotOperator(const Stmt *Statement) { - const auto *UnaryOperatorExpression = - llvm::dyn_cast<UnaryOperator>(Statement); - return UnaryOperatorExpression != nullptr && - UnaryOperatorExpression->getOpcode() == UO_LNot; + const auto *UnaryOperatorExpr = dyn_cast<UnaryOperator>(Statement); + return UnaryOperatorExpr && UnaryOperatorExpr->getOpcode() == UO_LNot; } bool areParensNeededForOverloadedOperator(OverloadedOperatorKind OperatorKind) { @@ -103,39 +100,35 @@ bool areParensNeededForOverloadedOperato } bool areParensNeededForStatement(const Stmt *Statement) { - if (const auto *OverloadedOperatorCall = - llvm::dyn_cast<CXXOperatorCallExpr>(Statement)) { - return areParensNeededForOverloadedOperator( - OverloadedOperatorCall->getOperator()); + if (const auto *OperatorCall = dyn_cast<CXXOperatorCallExpr>(Statement)) { + return areParensNeededForOverloadedOperator(OperatorCall->getOperator()); } - return llvm::isa<BinaryOperator>(Statement) || - llvm::isa<UnaryOperator>(Statement); + return isa<BinaryOperator>(Statement) || isa<UnaryOperator>(Statement); } -void addFixItHintsForGenericExpressionCastToBool( - DiagnosticBuilder &Diagnostic, const ImplicitCastExpr *CastExpression, - const Stmt *ParentStatement, ASTContext &Context) { +void fixGenericExprCastToBool(DiagnosticBuilder &Diag, + const ImplicitCastExpr *Cast, const Stmt *Parent, + ASTContext &Context) { // In case of expressions like (! integer), we should remove the redundant not // operator and use inverted comparison (integer == 0). bool InvertComparison = - ParentStatement != nullptr && isUnaryLogicalNotOperator(ParentStatement); + Parent != nullptr && isUnaryLogicalNotOperator(Parent); if (InvertComparison) { - SourceLocation ParentStartLoc = ParentStatement->getLocStart(); + SourceLocation ParentStartLoc = Parent->getLocStart(); SourceLocation ParentEndLoc = - llvm::cast<UnaryOperator>(ParentStatement)->getSubExpr()->getLocStart(); - Diagnostic.AddFixItHint(FixItHint::CreateRemoval( - CharSourceRange::getCharRange(ParentStartLoc, ParentEndLoc))); + cast<UnaryOperator>(Parent)->getSubExpr()->getLocStart(); + Diag << FixItHint::CreateRemoval( + CharSourceRange::getCharRange(ParentStartLoc, ParentEndLoc)); - auto FurtherParents = Context.getParents(*ParentStatement); - ParentStatement = FurtherParents[0].get<Stmt>(); + Parent = Context.getParents(*Parent)[0].get<Stmt>(); } - const Expr *SubExpression = CastExpression->getSubExpr(); + const Expr *SubExpr = Cast->getSubExpr(); - bool NeedInnerParens = areParensNeededForStatement(SubExpression); - bool NeedOuterParens = ParentStatement != nullptr && - areParensNeededForStatement(ParentStatement); + bool NeedInnerParens = areParensNeededForStatement(SubExpr); + bool NeedOuterParens = + Parent != nullptr && areParensNeededForStatement(Parent); std::string StartLocInsertion; @@ -147,9 +140,7 @@ void addFixItHintsForGenericExpressionCa } if (!StartLocInsertion.empty()) { - SourceLocation StartLoc = CastExpression->getLocStart(); - Diagnostic.AddFixItHint( - FixItHint::CreateInsertion(StartLoc, StartLocInsertion)); + Diag << FixItHint::CreateInsertion(Cast->getLocStart(), StartLocInsertion); } std::string EndLocInsertion; @@ -164,128 +155,91 @@ void addFixItHintsForGenericExpressionCa EndLocInsertion += " != "; } - EndLocInsertion += getZeroLiteralToCompareWithForGivenType( - CastExpression->getCastKind(), SubExpression->getType(), Context); + EndLocInsertion += getZeroLiteralToCompareWithForType( + Cast->getCastKind(), SubExpr->getType(), Context); if (NeedOuterParens) { EndLocInsertion += ")"; } SourceLocation EndLoc = Lexer::getLocForEndOfToken( - CastExpression->getLocEnd(), 0, Context.getSourceManager(), - Context.getLangOpts()); - Diagnostic.AddFixItHint(FixItHint::CreateInsertion(EndLoc, EndLocInsertion)); + Cast->getLocEnd(), 0, Context.getSourceManager(), Context.getLangOpts()); + Diag << FixItHint::CreateInsertion(EndLoc, EndLocInsertion); } -StringRef getEquivalentBoolLiteralForExpression(const Expr *Expression, - ASTContext &Context) { +StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, + ASTContext &Context) { if (isNULLMacroExpansion(Expression, Context)) { return "false"; } - if (const auto *IntLit = llvm::dyn_cast<IntegerLiteral>(Expression)) { + if (const auto *IntLit = dyn_cast<IntegerLiteral>(Expression)) { return (IntLit->getValue() == 0) ? "false" : "true"; } - if (const auto *FloatLit = llvm::dyn_cast<FloatingLiteral>(Expression)) { + if (const auto *FloatLit = dyn_cast<FloatingLiteral>(Expression)) { llvm::APFloat FloatLitAbsValue = FloatLit->getValue(); FloatLitAbsValue.clearSign(); return (FloatLitAbsValue.bitcastToAPInt() == 0) ? "false" : "true"; } - if (const auto *CharLit = llvm::dyn_cast<CharacterLiteral>(Expression)) { + if (const auto *CharLit = dyn_cast<CharacterLiteral>(Expression)) { return (CharLit->getValue() == 0) ? "false" : "true"; } - if (llvm::isa<StringLiteral>(Expression->IgnoreCasts())) { + if (isa<StringLiteral>(Expression->IgnoreCasts())) { return "true"; } return StringRef(); } -void addFixItHintsForLiteralCastToBool(DiagnosticBuilder &Diagnostic, - const ImplicitCastExpr *CastExpression, - StringRef EquivalentLiteralExpression) { - SourceLocation StartLoc = CastExpression->getLocStart(); - SourceLocation EndLoc = CastExpression->getLocEnd(); - - Diagnostic.AddFixItHint(FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(StartLoc, EndLoc), - EquivalentLiteralExpression)); -} - -void addFixItHintsForGenericExpressionCastFromBool( - DiagnosticBuilder &Diagnostic, const ImplicitCastExpr *CastExpression, - ASTContext &Context, StringRef OtherType) { - const Expr *SubExpression = CastExpression->getSubExpr(); - bool NeedParens = !llvm::isa<ParenExpr>(SubExpression); - - std::string StartLocInsertion = "static_cast<"; - StartLocInsertion += OtherType.str(); - StartLocInsertion += ">"; - if (NeedParens) { - StartLocInsertion += "("; - } - - SourceLocation StartLoc = CastExpression->getLocStart(); - Diagnostic.AddFixItHint( - FixItHint::CreateInsertion(StartLoc, StartLocInsertion)); +void fixGenericExprCastFromBool(DiagnosticBuilder &Diag, + const ImplicitCastExpr *Cast, + ASTContext &Context, StringRef OtherType) { + const Expr *SubExpr = Cast->getSubExpr(); + bool NeedParens = !isa<ParenExpr>(SubExpr); + + Diag << FixItHint::CreateInsertion( + Cast->getLocStart(), + (Twine("static_cast<") + OtherType + ">" + (NeedParens ? "(" : "")) + .str()); if (NeedParens) { SourceLocation EndLoc = Lexer::getLocForEndOfToken( - CastExpression->getLocEnd(), 0, Context.getSourceManager(), + Cast->getLocEnd(), 0, Context.getSourceManager(), Context.getLangOpts()); - Diagnostic.AddFixItHint(FixItHint::CreateInsertion(EndLoc, ")")); + Diag << FixItHint::CreateInsertion(EndLoc, ")"); } } -StringRef getEquivalentLiteralForBoolLiteral( - const CXXBoolLiteralExpr *BoolLiteralExpression, QualType DestinationType, - ASTContext &Context) { +StringRef getEquivalentForBoolLiteral(const CXXBoolLiteralExpr *BoolLiteral, + QualType DestType, ASTContext &Context) { // Prior to C++11, false literal could be implicitly converted to pointer. if (!Context.getLangOpts().CPlusPlus11 && - (DestinationType->isPointerType() || - DestinationType->isMemberPointerType()) && - BoolLiteralExpression->getValue() == false) { + (DestType->isPointerType() || DestType->isMemberPointerType()) && + BoolLiteral->getValue() == false) { return "0"; } - if (DestinationType->isFloatingType()) { - if (BoolLiteralExpression->getValue() == true) { - return Context.hasSameType(DestinationType, Context.FloatTy) ? "1.0f" - : "1.0"; + if (DestType->isFloatingType()) { + if (Context.hasSameType(DestType, Context.FloatTy)) { + return BoolLiteral->getValue() ? "1.0f" : "0.0f"; } - return Context.hasSameType(DestinationType, Context.FloatTy) ? "0.0f" - : "0.0"; + return BoolLiteral->getValue() ? "1.0" : "0.0"; } - if (BoolLiteralExpression->getValue() == true) { - return DestinationType->isUnsignedIntegerType() ? "1u" : "1"; + if (DestType->isUnsignedIntegerType()) { + return BoolLiteral->getValue() ? "1u" : "0u"; } - return DestinationType->isUnsignedIntegerType() ? "0u" : "0"; + return BoolLiteral->getValue() ? "1" : "0"; } -void addFixItHintsForLiteralCastFromBool(DiagnosticBuilder &Diagnostic, - const ImplicitCastExpr *CastExpression, - ASTContext &Context, - QualType DestinationType) { - SourceLocation StartLoc = CastExpression->getLocStart(); - SourceLocation EndLoc = CastExpression->getLocEnd(); - const auto *BoolLiteralExpression = - llvm::dyn_cast<CXXBoolLiteralExpr>(CastExpression->getSubExpr()); - - Diagnostic.AddFixItHint(FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(StartLoc, EndLoc), - getEquivalentLiteralForBoolLiteral(BoolLiteralExpression, DestinationType, - Context))); -} - -bool isAllowedConditionalCast(const ImplicitCastExpr *CastExpression, +bool isAllowedConditionalCast(const ImplicitCastExpr *Cast, ASTContext &Context) { std::queue<const Stmt *> Q; - Q.push(CastExpression); + Q.push(Cast); while (!Q.empty()) { for (const auto &N : Context.getParents(*Q.front())) { const Stmt *S = N.get<Stmt>(); @@ -294,8 +248,7 @@ bool isAllowedConditionalCast(const Impl if (isa<IfStmt>(S) || isa<ConditionalOperator>(S)) return true; if (isa<ParenExpr>(S) || isa<ImplicitCastExpr>(S) || - (isa<UnaryOperator>(S) && - cast<UnaryOperator>(S)->getOpcode() == UO_LNot) || + isUnaryLogicalNotOperator(S) || (isa<BinaryOperator>(S) && cast<BinaryOperator>(S)->isLogicalOp())) { Q.push(S); } else { @@ -371,69 +324,59 @@ void ImplicitBoolCastCheck::registerMatc void ImplicitBoolCastCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *CastToBool = Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastToBool")) { - const auto *ParentStatement = Result.Nodes.getNodeAs<Stmt>("parentStmt"); - return handleCastToBool(CastToBool, ParentStatement, *Result.Context); + const auto *Parent = Result.Nodes.getNodeAs<Stmt>("parentStmt"); + return handleCastToBool(CastToBool, Parent, *Result.Context); } if (const auto *CastFromBool = Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCastFromBool")) { - const auto *FurtherImplicitCastExpression = + const auto *NextImplicitCast = Result.Nodes.getNodeAs<ImplicitCastExpr>("furtherImplicitCast"); - return handleCastFromBool(CastFromBool, FurtherImplicitCastExpression, - *Result.Context); + return handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context); } } -void ImplicitBoolCastCheck::handleCastToBool( - const ImplicitCastExpr *CastExpression, const Stmt *ParentStatement, - ASTContext &Context) { +void ImplicitBoolCastCheck::handleCastToBool(const ImplicitCastExpr *Cast, + const Stmt *Parent, + ASTContext &Context) { if (AllowConditionalPointerCasts && - (CastExpression->getCastKind() == CK_PointerToBoolean || - CastExpression->getCastKind() == CK_MemberPointerToBoolean) && - isAllowedConditionalCast(CastExpression, Context)) { + (Cast->getCastKind() == CK_PointerToBoolean || + Cast->getCastKind() == CK_MemberPointerToBoolean) && + isAllowedConditionalCast(Cast, Context)) { return; } if (AllowConditionalIntegerCasts && - CastExpression->getCastKind() == CK_IntegralToBoolean && - isAllowedConditionalCast(CastExpression, Context)) { + Cast->getCastKind() == CK_IntegralToBoolean && + isAllowedConditionalCast(Cast, Context)) { return; } - std::string OtherType = CastExpression->getSubExpr()->getType().getAsString(); - DiagnosticBuilder Diagnostic = - diag(CastExpression->getLocStart(), "implicit cast '%0' -> bool") - << OtherType; - - StringRef EquivalentLiteralExpression = getEquivalentBoolLiteralForExpression( - CastExpression->getSubExpr(), Context); - if (!EquivalentLiteralExpression.empty()) { - addFixItHintsForLiteralCastToBool(Diagnostic, CastExpression, - EquivalentLiteralExpression); + auto Diag = diag(Cast->getLocStart(), "implicit cast %0 -> bool") + << Cast->getSubExpr()->getType(); + + StringRef EquivalentLiteral = + getEquivalentBoolLiteralForExpr(Cast->getSubExpr(), Context); + if (!EquivalentLiteral.empty()) { + Diag << tooling::fixit::createReplacement(*Cast, EquivalentLiteral); } else { - addFixItHintsForGenericExpressionCastToBool(Diagnostic, CastExpression, - ParentStatement, Context); + fixGenericExprCastToBool(Diag, Cast, Parent, Context); } } void ImplicitBoolCastCheck::handleCastFromBool( - const ImplicitCastExpr *CastExpression, - const ImplicitCastExpr *FurtherImplicitCastExpression, + const ImplicitCastExpr *Cast, const ImplicitCastExpr *NextImplicitCast, ASTContext &Context) { - QualType DestinationType = (FurtherImplicitCastExpression != nullptr) - ? FurtherImplicitCastExpression->getType() - : CastExpression->getType(); - std::string DestinationTypeString = DestinationType.getAsString(); - DiagnosticBuilder Diagnostic = - diag(CastExpression->getLocStart(), "implicit cast bool -> '%0'") - << DestinationTypeString; - - if (llvm::isa<CXXBoolLiteralExpr>(CastExpression->getSubExpr())) { - addFixItHintsForLiteralCastFromBool(Diagnostic, CastExpression, Context, - DestinationType); + QualType DestType = + NextImplicitCast ? NextImplicitCast->getType() : Cast->getType(); + auto Diag = diag(Cast->getLocStart(), "implicit cast bool -> %0") << DestType; + + if (const auto *BoolLiteral = + dyn_cast<CXXBoolLiteralExpr>(Cast->getSubExpr())) { + Diag << tooling::fixit::createReplacement( + *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); } else { - addFixItHintsForGenericExpressionCastFromBool( - Diagnostic, CastExpression, Context, DestinationTypeString); + fixGenericExprCastFromBool(Diag, Cast, Context, DestType.getAsString()); } } Modified: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp?rev=302160&r1=302159&r2=302160&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp Thu May 4 10:34:23 2017 @@ -40,7 +40,7 @@ void regularImplicitCastPointerToBoolIsN int Struct::* memberPointer = &Struct::member; functionTaking<bool>(memberPointer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int struct Struct::*' -> bool + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int Struct::*' -> bool // CHECK-FIXES: functionTaking<bool>(memberPointer != nullptr); } Modified: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp?rev=302160&r1=302159&r2=302160&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp Thu May 4 10:34:23 2017 @@ -20,7 +20,7 @@ void useOldNullMacroInReplacements() { int Struct::* memberPointer = NULL; functionTaking<bool>(!memberPointer); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast 'int struct Struct::*' -> bool + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast 'int Struct::*' -> bool // CHECK-FIXES: functionTaking<bool>(memberPointer == 0); } @@ -35,11 +35,11 @@ void fixFalseLiteralConvertingToNullPoin // CHECK-FIXES: if (pointer == 0) {} functionTaking<int Struct::*>(false); - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'int struct Struct::*' + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'int Struct::*' // CHECK-FIXES: functionTaking<int Struct::*>(0); int Struct::* memberPointer = NULL; if (memberPointer != false) {} - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int struct Struct::*' + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int Struct::*' // CHECK-FIXES: if (memberPointer != 0) {} } Modified: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp?rev=302160&r1=302159&r2=302160&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp Thu May 4 10:34:23 2017 @@ -195,7 +195,7 @@ void implicitCastToBoolSimpleCases() { auto pointerToMember = &Struct::member; functionTaking<bool>(pointerToMember); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int struct Struct::*' -> bool + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int Struct::*' -> bool // CHECK-FIXES: functionTaking<bool>(pointerToMember != nullptr); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits