[PATCH] D12734: Another patch for modernize-loop-convert.

[Manuel Klimek via cfe-commits]

klimek added inline comments.

================
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:458
@@ -438,2 +457,3 @@
     // variable.
     for (const auto &I : Usages) {
+      std::string ReplaceText;
----------------
I'd call that 'Usage' here, as it's not an iterator.

================
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:460-465
@@ -441,1 +459,8 @@
+      std::string ReplaceText;
+      if (I.Expression) {
+        ReplaceText = I.IsArrow ? VarName + "." : VarName;
+      } else {
+        // Lambda capture: IsArrow means that the index is captured by value.
+        ReplaceText = I.IsArrow ? "&" + VarName : VarName;
+      }
       TUInfo->getReplacedVars().insert(std::make_pair(TheLoop, IndexVar));
----------------
We need to document the members of Usage better: It seems unclear why the 'else' part goes into a lambda capture, or why in the if() path adding a '.' will ever help. Examples would help in comments here, I think.

================
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:765-766
@@ -764,2 +764,4 @@
       // exactly one usage.
-      addUsage(Usage(nullptr, false, C->getLocation()));
+      // We are using the 'IsArrow' field of Usage to store if we have to add
+      // a '&' to capture the element by reference.
+      addUsage(
----------------
This needs to be a comment at the IsArrow field of Usage.

================
Comment at: test/clang-tidy/modernize-loop-convert-basic.cpp:504
@@ -502,1 +503,3 @@
 
+void constPseudo() {
+  int sum = 0;
----------------
why Pseudo?


http://reviews.llvm.org/D12734