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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
