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

1 files changed, 32 insertions, 2 deletions

diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 11d5425956e..b6cb64fa952 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -364,6 +364,23 @@ static bool isDirectMemberExpr(const Expr *E) {
   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 @@ void LoopConvertCheck::doConversion(
   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 @@ void LoopConvertCheck::doConversion(
 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 @@ void LoopConvertCheck::findAndVerifyUsages(
     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);