alexfh added a comment. A few more notes, all fine for a follow up.
================ Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:202 + + const auto *LhsExpr = Result.Nodes.getNodeAs<Expr>("lhsExpr"); + const auto *RhsExpr = Result.Nodes.getNodeAs<Expr>("rhsExpr"); ---------------- Looks like you're doing exactly same thing twice for lhs and rhs. Pull this out to a function. Fine for a follow up. ================ Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:212-213 + RhsDecExpr ? dyn_cast<EnumConstantDecl>(RhsDecExpr->getDecl()) : nullptr; + bool LhsVar = !LhsConstant; + bool RhsVar = !RhsConstant; + ---------------- There's not much value in these variables. ================ Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:215-219 + int NonPowOfTwoCounter = countNonPowOfTwoLiteralNum(EnumDec); + ValueRange VR(EnumDec); + int EnumLen = enumLength(EnumDec); + if (!isPossiblyBitMask(NonPowOfTwoCounter, EnumLen, VR, EnumDec)) + return; ---------------- This code doesn't need the lhs/rhs variable declared above it. Move it up. ================ Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:220-227 + // Report left hand side parameter if neccessary. + if (LhsVar) { + diag(EnumDec->getInnerLocStart(), BitmaskVarErrorMessage); + diag(LhsExpr->getExprLoc(), BitmaskNoteMessage, DiagnosticIDs::Note); + } else if (isNonPowerOf2NorNullLiteral(LhsConstant)) { + diag(LhsConstant->getSourceRange().getBegin(), BitmaskErrorMessage); + diag(LhsExpr->getExprLoc(), BitmaskNoteMessage, DiagnosticIDs::Note); ---------------- This code can be pulled to a function / method to avoid repeating it twice (starting from the ` const auto *LhsExpr = Result.Nodes.getNodeAs<Expr>("lhsExpr");` part). ================ Comment at: docs/clang-tidy/checks/misc-suspicious-enum-usage.rst:31 + +=============== + ---------------- alexfh wrote: > This notation is used to make the previous line a heading. Doesn't seem to be > the case here. See http://llvm.org/docs/SphinxQuickstartTemplate.html for > some examples. Please also try to build your docs to check for obvious issues. This is not done yet. https://reviews.llvm.org/D22507 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits