steveire updated this revision to Diff 321997.
steveire added a comment.
Update
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96142/new/
https://reviews.llvm.org/D96142
Files:
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone-too-small-loop-variable.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-too-small-loop-variable.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-too-small-loop-variable.cpp
@@ -65,7 +65,19 @@
template <class T>
void doSomething() {
for (T i = 0; i < size(); ++i) {
- // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable is template dependent and might be instantiated with a type which is too small.
+ }
+}
+
+template <class T>
+struct Templ {
+};
+
+template <class T>
+void dependentBoundary() {
+ Templ<T> t;
+ for (int i = 0; i < t.size(); ++i) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: boundary variable is template dependent and might be instantiated with a type which is too large for the loop variable.
}
}
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
@@ -34,6 +34,9 @@
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
private:
const unsigned MagnitudeBitsUpperLimit;
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -20,8 +20,6 @@
llvm::StringLiteral("forLoopName");
static constexpr llvm::StringLiteral LoopVarName =
llvm::StringLiteral("loopVar");
-static constexpr llvm::StringLiteral LoopVarCastName =
- llvm::StringLiteral("loopVarCast");
static constexpr llvm::StringLiteral LoopUpperBoundName =
llvm::StringLiteral("loopUpperBound");
static constexpr llvm::StringLiteral LoopIncrementName =
@@ -45,50 +43,38 @@
/// \endcode
/// The following string identifiers are bound to these parts of the AST:
/// LoopVarName: 'j' (as a VarDecl)
-/// LoopVarCastName: 'j' (after implicit conversion)
/// LoopUpperBoundName: '3 + 2' (as an Expr)
/// LoopIncrementName: 'k' (as an Expr)
/// LoopName: The entire for loop (as a ForStmt)
///
void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
StatementMatcher LoopVarMatcher =
- expr(
- ignoringParenImpCasts(declRefExpr(to(varDecl(hasType(isInteger()))))))
+ declRefExpr(anyOf(hasType(isInteger()), isTypeDependent()))
.bind(LoopVarName);
- // We need to catch only those comparisons which contain any integer cast.
- StatementMatcher LoopVarConversionMatcher = traverse(
- TK_AsIs, implicitCastExpr(hasImplicitDestinationType(isInteger()),
- has(ignoringParenImpCasts(LoopVarMatcher)))
- .bind(LoopVarCastName));
-
- // We are interested in only those cases when the loop bound is a variable
- // value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())))))
+ auto LoopBoundMatcher =
+ expr(anyOf(allOf(hasType(isInteger()), unless(integerLiteral()),
+ unless(hasType(isConstQualified())),
+ unless(hasType(enumType()))),
+ isTypeDependent()))
.bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
- StatementMatcher IncrementMatcher =
- expr(ignoringParenImpCasts(hasType(isInteger()))).bind(LoopIncrementName);
+ auto IncrementMatcher = expr(anyOf(hasType(isInteger()), isTypeDependent()))
+ .bind(LoopIncrementName);
Finder->addMatcher(
forStmt(
hasCondition(anyOf(
- binaryOperator(hasOperatorName("<"),
- hasLHS(LoopVarConversionMatcher),
+ binaryOperator(hasOperatorName("<"), hasLHS(LoopVarMatcher),
hasRHS(LoopBoundMatcher)),
- binaryOperator(hasOperatorName("<="),
- hasLHS(LoopVarConversionMatcher),
+ binaryOperator(hasOperatorName("<="), hasLHS(LoopVarMatcher),
hasRHS(LoopBoundMatcher)),
binaryOperator(hasOperatorName(">"), hasLHS(LoopBoundMatcher),
- hasRHS(LoopVarConversionMatcher)),
+ hasRHS(LoopVarMatcher)),
binaryOperator(hasOperatorName(">="), hasLHS(LoopBoundMatcher),
- hasRHS(LoopVarConversionMatcher)))),
+ hasRHS(LoopVarMatcher)))),
hasIncrement(IncrementMatcher))
.bind(LoopName),
this);
@@ -150,6 +136,19 @@
const auto *LoopIncrement =
Result.Nodes.getNodeAs<Expr>(LoopIncrementName)->IgnoreParenImpCasts();
+ if (LoopVar->isTypeDependent()) {
+ diag(LoopVar->getBeginLoc(),
+ "loop variable is template dependent and might be instantiated with a "
+ "type which is too small.");
+ return;
+ }
+ if (UpperBound->isTypeDependent()) {
+ diag(UpperBound->getBeginLoc(),
+ "boundary variable is template dependent and might be instantiated "
+ "with a type which is too large for the loop variable.");
+ return;
+ }
+
// We matched the loop variable incorrectly.
if (LoopVar->getType() != LoopIncrement->getType())
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
