alexfh added a comment.
A few minor comments.
================
Comment at: clang-tidy/modernize/LoopConvertCheck.h:68
@@ -67,2 +67,3 @@
Confidence::Level MinConfidence;
+ VariableNamer::NamingStyle NamingStyle;
};
----------------
The variable can be const, the one above as well.
================
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:820
@@ -822,1 +819,3 @@
+ size_t Len = ContainerName.size();
+ if (Len > 1 && ContainerName.endswith(Style == NS_UpperCase ? "S" : "s")) {
IteratorName = ContainerName.substr(0, Len - 1);
----------------
No need to check the length. `endswith` handles this itself.
================
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:841
@@ -828,4 +840,3 @@
- IteratorName = ContainerName + "_" + OldIndex->getName().str();
- if (!declarationExists(IteratorName))
- return IteratorName;
+ auto AddSuffixToContainer = [&](std::string Suffix) -> std::string {
+ std::string Name = ContainerName;
----------------
Automatic captures always look suspicious and require more attention. I'd avoid
them unless there's a significant improvement in readability. I'd also prefer a
more generic function, say `AppendSuffixWithStyle`, which could then be placed
next to the styles enumeration and other functions to work with banking styles.
================
Comment at: clang-tidy/modernize/LoopConvertUtils.h:453
@@ -443,2 +452,3 @@
bool declarationExists(llvm::StringRef Symbol);
+
};
----------------
nit: please remove the empty line.
http://reviews.llvm.org/D13052
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
