djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
A few nitpicks, but otherwise looks good.
================
Comment at: lib/Format/Format.cpp:1287
@@ +1286,3 @@
+ int Ret = INT_MAX;
+ for (unsigned I = 0, E = CategoryRegexs.size(); I != E; ++I)
+ if (CategoryRegexs[I].match(IncludeName)) {
----------------
I'd consistently try to use i and e for integers vs. I and E for iterators.
================
Comment at: unittests/Format/CleanupTest.cpp:256
@@ +255,3 @@
+ inline std::string apply(StringRef Code, const tooling::Replacements
Replaces,
+ const FormatStyle &Style = getLLVMStyle()) {
+ return applyAllReplacements(
----------------
I'd probably make "Style" a class member, defaulting to LLVM Style that you can
then overwrite for other styles.
================
Comment at: unittests/Format/CleanupTest.cpp:270
@@ +269,3 @@
+
+ int getOffset(StringRef Code, int Line, int Column) {
+ RewriterTestContext Context;
----------------
I'd consider making "Code" a member variable. But it has pros and cons.
================
Comment at: unittests/Format/CleanupTest.cpp:313
@@ +312,3 @@
+TEST_F(CleanUpReplacementsTest, NoExistingIncludeWithDefine) {
+ std::string Code = "#ifndef __A_H__\n"
+ "#define __A_H__\n"
----------------
Why are you starting and ending the header guards with a double underscore? I
think this is actually reserved for built-in identifiers. Maybe just stick with
the LLVM naming scheme?
http://reviews.llvm.org/D20734
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
