Fix several corner cases for loop-convert check.
Summary: Reduced the amount of wrong conversions of this check.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D12530
llvm-svn: 246550
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 11d5425..b6cb64f 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -364,6 +364,23 @@
return false;
}
+/// \brief Returns true when it can be guaranteed that the elements of the
+/// container are not being modified.
+static bool usagesAreConst(const UsageResult &Usages) {
+ // FIXME: Make this function more generic.
+ return Usages.empty();
+}
+
+/// \brief Returns true if the elements of the container are never accessed
+/// by reference.
+static bool usagesReturnRValues(const UsageResult &Usages) {
+ for (const auto &U : Usages) {
+ if (!U.Expression->isRValue())
+ return false;
+ }
+ return true;
+}
+
LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
MinConfidence(StringSwitch<Confidence::Level>(
@@ -452,7 +469,8 @@
StringRef MaybeDereference = ContainerNeedsDereference ? "*" : "";
std::string TypeString = AutoRefType.getAsString();
std::string Range = ("(" + TypeString + " " + VarName + " : " +
- MaybeDereference + ContainerString + ")").str();
+ MaybeDereference + ContainerString + ")")
+ .str();
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(ParenRange), Range);
TUInfo->getGeneratedDecls().insert(make_pair(TheLoop, VarName));
@@ -464,7 +482,7 @@
StringRef LoopConvertCheck::checkRejections(ASTContext *Context,
const Expr *ContainerExpr,
const ForStmt *TheLoop) {
- // If we already modified the reange of this for loop, don't do any further
+ // If we already modified the range of this for loop, don't do any further
// updates on this iteration.
if (TUInfo->getReplacedVars().count(TheLoop))
return "";
@@ -525,6 +543,18 @@
if (!getReferencedVariable(ContainerExpr) &&
!isDirectMemberExpr(ContainerExpr))
ConfidenceLevel.lowerTo(Confidence::CL_Risky);
+ } else if (FixerKind == LFK_PseudoArray) {
+ if (!DerefByValue && !DerefByConstRef) {
+ const UsageResult &Usages = Finder.getUsages();
+ if (usagesAreConst(Usages)) {
+ // FIXME: check if the type is trivially copiable.
+ DerefByConstRef = true;
+ } else if (usagesReturnRValues(Usages)) {
+ // If the index usages (dereference, subscript, at) return RValues,
+ // then we should not use a non-const reference.
+ DerefByValue = true;
+ }
+ }
}
StringRef ContainerString = checkRejections(Context, ContainerExpr, TheLoop);